Create linting plugin that prevents non-module imports in *.py files
Add a linting plugin to pylint to ensure that only modules are imported
How can I link https://github.com/quantumlib/Cirq/issues/4863 with this PR?
Hm, I understood #4863 a bit differently, with the first priority being to organize and sort imports (3 groups for stdlib, then third party, then cirq imports, with alphabetical sorting in each group). In addition to that I think it's nice to normalize the import style, e.g. internally we use import foo.bar.baz as bas for importing modules and from foo.bar.baz import Qux for importing names from modules. While it's good to discourage the latter, I think it'll be quite difficult to disallow these entirely.
Ok, I'm going to ask a few questions to figure out the precise scope of what this PR should do.
Is there an automated formatting script (as mentioned in #4863) that automatically formats PRs? Or is it a script that is run while doing local development?
What is the organization of imports going to look like? Will it be 3 groups, where the groups look like (in order):
- stdlib imports (alphabetized)
- 3rd party imports (alphabetized)
- cirq imports (alphabetized)
As for the work done in this PR, should it be removed completely? Or should we generate a warning when a non-module entity is imported?
Thanks in advance!
Is there an automated formatting script (as mentioned in https://github.com/quantumlib/Cirq/issues/4863) that automatically formats PRs? Or is it a script that is run while doing local development?
We have a script check/format-incremental that can be run locally to format files and that also runs as part of ci to ensure that things are formatted before allowing a merge (https://github.com/quantumlib/Cirq/blob/master/.github/workflows/ci.yml#L56). We can add additional step there, such as running isort to cleanup imports.
What is the organization of imports going to look like? Will it be 3 groups, where the groups look like (in order):
- stdlib imports (alphabetized)
- 3rd party imports (alphabetized)
- cirq imports (alphabetized)
Yeah, that looks right. I think this is basically what isort does by default. Internally we use the following configuration in pyproject.toml for isort:
[tool.isort]
profile = 'black'
line_length = 100
order_by_type = false # Sort alphabetically, irrespective of case.
remove_redundant_aliases = true
As for the work done in this PR, should it be removed completely? Or should we generate a warning when a non-module entity is imported?
I'm not sure. I think we should prioritize automatic import sorting, but beyond that I'd be curious what other people think about how strictly we want to enforce things like only importing modules.