Flake8 (Hound CI) rule E402 breaks AppVeyor build

Developer's Area
1

(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
2

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:

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

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

 flake8:
   enabled: true
+  config_file: .flake8
3

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.

4

Flake8 (Hound CI) rule E402 breaks AppVeyor build · Issue #919 · mypaint/mypaint · GitHub 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.

5

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: