Flake8 (Hound CI) rule E402 breaks AppVeyor build

(Not sure if this belonged on the github issue tracker)

There are 2 github PR checks that conflict with each other. Hound CI and AppVeyor.

Hound CI (which uses Flake8) rule E402 is:

module level import not at top of file

But if you put the import at the top of the file it will appear before the one from __future__, which has to be at the top according to the AppVeyor build (see here: Build Log, scroll to bottom, look for the Traceback).

I suppose AppVeyor is right here, since this __future__ import seems to do some meta-programming behaviour? It’s not just a linting check anyway.

This rule should probably be disabled altogether, but you can suppress it with a comment like:

from . import brushmanager  # noqa: E402

I think a .flake8 file can be added to the repo with an ignore = E402 to do that. I’ve tested the following file locally:

# see https://lintlyci.github.io/Flake8Rules/ for a list of all rules
ignore =
	# module level import not at top of file

Which seems to do the job.

And then add this file to .hound.yml:

--- a/.hound.yml
+++ b/.hound.yml
@@ -7,3 +7,4 @@ fail_on_violations: true

   enabled: true
+  config_file: .flake8

I would go ahead and submit a github issue as well since this dose break stuff. Just link back to this thread so you don’t have to re-explain why and possible fix.

https://github.com/mypaint/mypaint/issues/919 Done!

I was cautious, since I got it wrong before. The issue template on github seems to be focussed solely on bug reports for problems with the usage of MyPaint.

It’s reasonable to report potential build issues there. Just replace the template if it doesn’t really match the issue you’e noticed.

This one looks like it was just __future__ not being imported first, but I’m doing an isolated build on a test branch with just some linter-suggested changes in it to be a bit more certain.

Hound-CI shouldn’t yelp about a from __future__ import ... being a violation of rule E402! If it’ making a noise, it’s not doing it’s job right because that’s an important Python syntactic construct :frowning_face:

I kinda want to keep the import order checking in there, because I want to be yapped at abound certain things. The current imports are a bit of a mess, and default rules are mostly sensible. It’s acceptable pain, to me :slight_smile: