Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constant Name (C0103) PEP 8 Misinterpretation #3111

Closed
rcgoodfellow opened this issue Sep 21, 2019 · 2 comments
Closed

Constant Name (C0103) PEP 8 Misinterpretation #3111

rcgoodfellow opened this issue Sep 21, 2019 · 2 comments
Labels
Enhancement ✨ Improvement to a component

Comments

@rcgoodfellow
Copy link

rcgoodfellow commented Sep 21, 2019

Pylint seems to take the statement

Constants are usually defined on a module level and written in all capital letters with underscores separating words.

To mean that all module level variables are constants, which is not what the above is saying. It's saying that if something is a constant, then it is usually at module level and written in all capitals. A reasonable interpretation of this is that constants should be written in all capitals and placed at module level, but not that all module level variables are constants.

The current pylint interpretation results in rampant capitalization ad nauseam in places where it absolutely does not belong, especially when folks run pylint over simple scripts. I do not see any mention in PEP 8 of capitalizing module or script level variables simply due to their scope.

@rcgoodfellow rcgoodfellow changed the title Invalid Constant Name (C0103) PEP 8 Misinterpretation Constant Name (C0103) PEP 8 Misinterpretation Sep 21, 2019
@PCManticore
Copy link
Contributor

I think you are right, this was badly interpreted since the beginnings of this check. It's definitely something we should change in 2.5.

@PCManticore PCManticore added the Enhancement ✨ Improvement to a component label Sep 26, 2019
@rcgoodfellow
Copy link
Author

Thanks!

pastelmind added a commit to pastelmind/d2txt that referenced this issue Jan 21, 2020
This check causes many false positives, especially for global variables
used in simple scripts.

Looks like this will be fixed in Pylint 2.5.0, though. It's already been
integrated into the main branch:
pylint-dev/pylint@3422e4a

Also see relevant issues that motivated the change:
- pylint-dev/pylint#3111
- pylint-dev/pylint#3132

The latest changelog with the fix mentioned under 2.5.0 changes:
https://github.com/PyCQA/pylint/blob/f2f4e6f42416644471ab003d4df7ecf052c3c411/ChangeLog
pastelmind added a commit to pastelmind/d2txt that referenced this issue Jan 21, 2020
Move all CLI script code under `if __name__ == "__main__"` to separate
module-level `main()` methods that serve as the entrypoint. This fixes
many invalid-name errors and all redefined-outer-name errors.

Sort imports alphabetically, and group them in the order:
builtins, 3rd-parties, local stuff. This eliminates import-related
warnings.

Other minor warnings are dealt with by fixing the code, or disabling the
warning if it is a false positive.

Notably, refactor warnings (Rxxxx) are not handled. These require
considerably more thought to work out. Since VS code doesn't show them
as errors, we're in no rush.

Unrelated to Pylint warnings: In sample scripts, remove the statements
that directly added the project root to sys.path. This was a hack used
to make d2txt.py importable in sample scripts. Now that we have
`pip install -e .`, there's no need to tamper with sys.path.

Note: As of Pylint 2.4.4, it incorrectly believes that all module-level
variables are constants, and emits invalid-name on them. This check had
to be disabled via comments sprinkled throughout the codebase.
Looks like this will be fixed in Pylint 2.5.0, though. It's already been
integrated into the main branch:
pylint-dev/pylint@3422e4a

Also see relevant issues that motivated the change:
- pylint-dev/pylint#3111
- pylint-dev/pylint#3132

The latest changelog with the fix mentioned under 2.5.0 changes:
https://github.com/PyCQA/pylint/blob/f2f4e6f42416644471ab003d4df7ecf052c3c411/ChangeLog
pastelmind added a commit to pastelmind/d2txt that referenced this issue Jan 22, 2020
Move all CLI script code under `if __name__ == "__main__"` to separate
module-level `main()` methods that serve as the entrypoint. This fixes
many invalid-name errors and all redefined-outer-name errors.

Sort imports alphabetically, and group them in the order:
builtins, 3rd-parties, local stuff. This eliminates import-related
warnings.

Other minor warnings are dealt with by fixing the code, or disabling the
warning if it is a false positive.

Notably, refactor warnings (Rxxxx) are not handled. These require
considerably more thought to work out. Since VS code doesn't show them
as errors, we're in no rush.

Unrelated to Pylint warnings: In sample scripts, remove the statements
that directly added the project root to sys.path. This was a hack used
to make d2txt.py importable in sample scripts. Now that we have
`pip install -e .`, there's no need to tamper with sys.path.

Note: As of Pylint 2.4.4, it incorrectly believes that all module-level
variables are constants, and emits invalid-name on them. This check had
to be disabled via comments sprinkled throughout the codebase.
Looks like this will be fixed in Pylint 2.5.0, though. It's already been
integrated into the main branch:
pylint-dev/pylint@3422e4a

Also see relevant issues that motivated the change:
- pylint-dev/pylint#3111
- pylint-dev/pylint#3132

The latest changelog with the fix mentioned under 2.5.0 changes:
https://github.com/PyCQA/pylint/blob/f2f4e6f42416644471ab003d4df7ecf052c3c411/ChangeLog
pastelmind added a commit to pastelmind/d2txt that referenced this issue Jan 22, 2020
Move all CLI script code under `if __name__ == "__main__"` to separate
module-level `main()` methods that serve as the entrypoint. This fixes
many invalid-name errors and all redefined-outer-name errors.

Sort imports alphabetically, and group them in the order:
builtins, 3rd-parties, local stuff. This eliminates import-related
warnings.

Other minor warnings are dealt with by fixing the code, or disabling the
warning if it is a false positive.

Notably, refactor warnings (Rxxxx) are not handled. These require
considerably more thought to work out. Since VS code doesn't show them
as errors, we're in no rush.

Unrelated to Pylint warnings: In sample scripts, remove the statements
that directly added the project root to sys.path. This was a hack used
to make d2txt.py importable in sample scripts. Now that we have
`pip install -e .`, there's no need to tamper with sys.path.

Note: As of Pylint 2.4.4, it incorrectly believes that all module-level
variables are constants, and emits invalid-name on them. This check had
to be disabled via comments sprinkled throughout the codebase.
Looks like this will be fixed in Pylint 2.5.0, though. It's already been
integrated into the main branch:
pylint-dev/pylint@3422e4a

Also see relevant issues that motivated the change:
- pylint-dev/pylint#3111
- pylint-dev/pylint#3132

The latest changelog with the fix mentioned under 2.5.0 changes:
https://github.com/PyCQA/pylint/blob/f2f4e6f42416644471ab003d4df7ecf052c3c411/ChangeLog
pastelmind added a commit to pastelmind/d2txt that referenced this issue Jan 22, 2020
Move all CLI script code under `if __name__ == "__main__"` to separate
module-level `main()` methods that serve as the entrypoint. This fixes
many invalid-name errors and all redefined-outer-name errors.

Sort imports alphabetically, and group them in the order:
builtins, 3rd-parties, local stuff. This eliminates import-related
warnings.

Other minor warnings are dealt with by fixing the code, or disabling the
warning if it is a false positive.

Notably, refactor warnings (Rxxxx) are not handled. These require
considerably more thought to work out. Since VS code doesn't show them
as errors, we're in no rush.

Unrelated to Pylint warnings: In sample scripts, remove the statements
that directly added the project root to sys.path. This was a hack used
to make d2txt.py importable in sample scripts. Now that we have
`pip install -e .`, there's no need to tamper with sys.path.

Note: As of Pylint 2.4.4, it incorrectly believes that all module-level
variables are constants, and emits invalid-name on them. This check had
to be disabled via comments sprinkled throughout the codebase.
Looks like this will be fixed in Pylint 2.5.0, though. It's already been
integrated into the main branch:
pylint-dev/pylint@3422e4a

Also see relevant issues that motivated the change:
- pylint-dev/pylint#3111
- pylint-dev/pylint#3132

The latest changelog with the fix mentioned under 2.5.0 changes:
https://github.com/PyCQA/pylint/blob/f2f4e6f42416644471ab003d4df7ecf052c3c411/ChangeLog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

No branches or pull requests

2 participants