[ci] add flake8

[jameslamb]

Is your feature request related to a problem? Please describe.

The project does not currently have any automatic protections against some classes of issue that can be detected by flake8, including:

• unused imports
• unused variables
• duplicated test names (which leads to pytest skipping tests)
• f-strings used on strings with no templating

Describe the solution you'd like

I believe this project should use flake8 for linting, and should store the configuration for that tool in a setup.cfg file (https://flake8.pycqa.org/en/latest/user/configuration.html).

Adding this tool to the project's testing setup would reduce the effort required for pull request authors and reviewers to detect such issues, and would reduce the risk of code changes with such issues making it to main.

Describe alternatives you've considered

N/A

Additional context

Here is the result of running flake8 (ignoring style-only issues) on the current latest commit on main.

flake8 \
    --ignore=E126,E128,E203,E241,E261,E302,E303,E402,E501,W503,W504,W605 \
    .

./hamilton/function_modifiers.py:50:13: F402 import 'node' from line 11 shadowed by loop variable
./hamilton/function_modifiers.py:169:30: F541 f-string is missing placeholders
./hamilton/function_modifiers.py:874:21: F541 f-string is missing placeholders
./hamilton/function_modifiers.py:878:34: F541 f-string is missing placeholders
./hamilton/graph.py:172:53: F821 undefined name 'graphviz'
./hamilton/graph.py:195:92: F821 undefined name 'networkx'
./hamilton/graph.py:316:13: F401 'graphviz' imported but unused
./hamilton/graph.py:451:17: F841 local variable 'e' is assigned to but never used
./hamilton/node.py:3:1: F401 'typing.Collection' imported but unused
./hamilton/driver.py:24:5: F811 redefinition of unused 'node' from line 13
./hamilton/data_quality/default_validators.py:227:30: F541 f-string is missing placeholders
./hamilton/data_quality/default_validators.py:394:9: F401 'pandera' imported but unused
./hamilton/data_quality/default_validators.py:396:21: F541 f-string is missing placeholders
./hamilton/data_quality/pandera_validators.py:1:1: F401 'typing.Any' imported but unused
./hamilton/data_quality/pandera_validators.py:21:16: F541 f-string is missing placeholders
./tests/test_function_modifiers.py:620:1: F811 redefinition of unused 'test_tags_invalid_value' from line 607
./tests/resources/bad_functions.py:5:1: F401 'tests.resources.only_import_me' imported but unused
./tests/resources/layered_decorators.py:1:1: F401 'hamilton.function_modifiers.tag' imported but unused
./tests/resources/cyclic_functions.py:5:1: F401 'tests.resources.only_import_me' imported but unused
./tests/resources/data_quality.py:1:1: F401 'numpy as np' imported but unused
./examples/ray/hello_world/run_rayworkflow.py:1:1: F401 'importlib' imported but unused
./graph_adapter_tests/h_ray/test_h_ray_workflow.py:1:1: F401 'tempfile' imported but unused

If maintainers are interested, I'd be happy to put up a pull request proposing this.

Thanks for your time and consideration.

[skrawcz]

@jameslamb yeah I think some of those warnings are invalid, for others you're correct. As long as we can annotate that certain cases should be skipped then I think this would be useful to run as a pre-commit hook in addition to adding it to circleci.

But, let me work on reformatting to black first #119 .

[jameslamb]

yep for sure! flake8 supports line-level exclusions (to suppress warnings about individual false positives), file-level exclusions, and can be configured to skip any warnings you don't care about.

For example, I like the pattern of using black for style and ignoring all the flake8 style things, and just having it focus on those other correctness, efficiency, and safety things.

[skrawcz]

Sorry @jameslamb let me add black to my TODO list for this week/next week so I can unblock this.

Issue for reference #174

[jameslamb]

no prob no prob

[skrawcz]

FYI @jameslamb #184 by @elijahbenizzy will be merged tomorrow (🤞).

[skrawcz]

@jameslamb you're good.

[jameslamb]

I put up a few PRs tonight that together address all the flake8 warnings that I'm confident aren't false positives.

• remove unused imports #218
• fix a few flake8 warnings #220
• remove unnecessary code in tests #221

If / when those are merged, I'll put up one more that adds flake8 to pre-commit and CI, and adds # noqa comments telling it which lines to ignore.