Adapt file headers to Lint specifications #3127
Labels
type: enhancement
It's working, but needs polishing
Comments
|
I have never seen this issue using |
|
It appears when I use |
|
@rafamartinc What platform are you testing on? |
|
On Windows 10. It was just easier to use |
|
@rafamartinc this is a pretty common issue with pylint (one of many reasons I opened #1179). Pylint is very sensitive to the environment it's run in. The only way to get pylint to behave consistently is to pin pylint and anything packages or tools that it uses to an exact version. We've been doing this in the constraints file, but we're likely missing several packages there. But even with pinning pylint and astroid it still behaves differently for me between my desktop, laptop, and CI (although I've never experienced an issue with the header rule check). Assuming you only have one version of python3 installed on your local system can you run |
|
Oh, and the other key difference I forgot is python version. We run in CI with python 3.5 (although I'm going to push a PR to update that to 3.7 shortly) and pylint will flag errors based on different versions (mostly because of stdlib differences, but sometimes for more subtle reasons). For example, the most recent example is on python 3.7 (which is my local default) lint will fail with: because 3.7 changed the signature for |
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this issue
Oct 3, 2019
The python version used for pylint can effect results returned, pylint is very sensitive to the environment it's run in. Running on python 3.5 which is the oldest supported python version masks issues on newer python versions. For example, if you have python 3.7 installed locally lint will always fail becaues of changes to the stdlib. This starts to become more of an issue as a particular python version ages, since the user and contributor base moves to newer versions of python. This commit changes the version of pylint we run in CI to 3.7 to tests the leading edge instead of the trailing so we keep our pylint testing up to date with the python versions we support. Related to Qiskit#3127 and Qiskit#1179
kdk
pushed a commit
that referenced
this issue
Oct 9, 2019
* Bump lint jobs to latest stable python version The python version used for pylint can effect results returned, pylint is very sensitive to the environment it's run in. Running on python 3.5 which is the oldest supported python version masks issues on newer python versions. For example, if you have python 3.7 installed locally lint will always fail becaues of changes to the stdlib. This starts to become more of an issue as a particular python version ages, since the user and contributor base moves to newer versions of python. This commit changes the version of pylint we run in CI to 3.7 to tests the leading edge instead of the trailing so we keep our pylint testing up to date with the python versions we support. Related to #3127 and #1179 * Add **kwargs to avoid linter error The abc.ABCMeta class defition added a **kwargs to the __new__ method definition in python 3.6. This breaks the linter because the qiskit.pulse.commands.command.MetaCount subclass function signature differs from stdlib on newer python versions. This works around this by adding a throwaway kwargs argument to the signature so that we pass pylint's blind pedantry when running on newer python versions, but still work with python 3.5.
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this issue
Jun 1, 2020
Currently we're relying on a pylint plugin to verify copyright headers conform to the project guidelines. However this is problematic for a number of reasons. The first is it has proven to be unstable there have been several instances of both users and CI failing on header files that do not have any issues. This is pretty common with pylint in general since it's very sensitive to differences in environment. Additionally, it was using a regex to try and match the entire header string. This provided no practical debugability when it fails (especially when the failure is unexpected) because we end up with only a match error. This commit addresses these issues by removing the use of the regex based pylint plugin and adding a small script to verify the header in every qiskit file and provide useful debugging if there is an issue with a header in a python file. Fixes Qiskit#3127
1ucian0
pushed a commit
that referenced
this issue
Jun 16, 2020
* Add tools/verify_headers.py to check copyright headers Currently we're relying on a pylint plugin to verify copyright headers conform to the project guidelines. However this is problematic for a number of reasons. The first is it has proven to be unstable there have been several instances of both users and CI failing on header files that do not have any issues. This is pretty common with pylint in general since it's very sensitive to differences in environment. Additionally, it was using a regex to try and match the entire header string. This provided no practical debugability when it fails (especially when the failure is unexpected) because we end up with only a match error. This commit addresses these issues by removing the use of the regex based pylint plugin and adding a small script to verify the header in every qiskit file and provide useful debugging if there is an issue with a header in a python file. Fixes #3127 * Add toxinidir to tox script call * Explicitly set character encoding for windows In manual testing running the verify_headers.py script on windows would fail because it was trying to use the system default character encoding instead of utf8 and would encounter chartacters outside the windows charmap encoding. To avoid this issue this commit explicitly sets the character encoding to utf8. We already declare that in the header (which is part of what this script is verifying) so we can assume all files are utf8 encoded and if they're not the script will catch it. Co-authored-by: Luciano Bello <[email protected]>
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this issue
Aug 5, 2020
* Bump lint jobs to latest stable python version The python version used for pylint can effect results returned, pylint is very sensitive to the environment it's run in. Running on python 3.5 which is the oldest supported python version masks issues on newer python versions. For example, if you have python 3.7 installed locally lint will always fail becaues of changes to the stdlib. This starts to become more of an issue as a particular python version ages, since the user and contributor base moves to newer versions of python. This commit changes the version of pylint we run in CI to 3.7 to tests the leading edge instead of the trailing so we keep our pylint testing up to date with the python versions we support. Related to Qiskit#3127 and Qiskit#1179 * Add **kwargs to avoid linter error The abc.ABCMeta class defition added a **kwargs to the __new__ method definition in python 3.6. This breaks the linter because the qiskit.pulse.commands.command.MetaCount subclass function signature differs from stdlib on newer python versions. This works around this by adding a throwaway kwargs argument to the signature so that we pass pylint's blind pedantry when running on newer python versions, but still work with python 3.5.
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this issue
Aug 5, 2020
* Add tools/verify_headers.py to check copyright headers Currently we're relying on a pylint plugin to verify copyright headers conform to the project guidelines. However this is problematic for a number of reasons. The first is it has proven to be unstable there have been several instances of both users and CI failing on header files that do not have any issues. This is pretty common with pylint in general since it's very sensitive to differences in environment. Additionally, it was using a regex to try and match the entire header string. This provided no practical debugability when it fails (especially when the failure is unexpected) because we end up with only a match error. This commit addresses these issues by removing the use of the regex based pylint plugin and adding a small script to verify the header in every qiskit file and provide useful debugging if there is an issue with a header in a python file. Fixes Qiskit#3127 * Add toxinidir to tox script call * Explicitly set character encoding for windows In manual testing running the verify_headers.py script on windows would fail because it was trying to use the system default character encoding instead of utf8 and would encounter chartacters outside the windows charmap encoding. To avoid this issue this commit explicitly sets the character encoding to utf8. We already declare that in the header (which is part of what this script is verifying) so we can assume all files are utf8 encoded and if they're not the script will catch it. Co-authored-by: Luciano Bello <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is the expected enhancement?
When running the test suite, specifically 'lint' tests for code quality, the same message is displayed for every file in the project:
************* Module qiskit.tools.visualization qiskit\tools\visualization.py:1:0: C5001: File header should match regex "(?:(?:#[^\n]*)?\n)*# This code is part of Qiskit.\n#\n# \(C\) Copyright IBM [0-9, -]*.\n#\n# This code is licensed under the Apache License, Version 2.0. You may\n# obtain a copy of this license in the LICENSE.txt file in the root directory\n# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.\n#\n# Any modifications or derivative works of this code must retain this\n# copyright notice, and modified files need to carry a notice indicating\n# that they have been altered from the originals.\n" (invalid-file-header)This message, with some variations, appears hundreds of times in the log. That makes it a bit difficult to keep track of the other code smells that should be fixed. Setting a new format for the headers, or making Lint ignore this issue, would remove hundreds of log entries in one go.
The text was updated successfully, but these errors were encountered: