A proliferation of (un-)Pythonically programmatic pragmas
I love many of the ancillary tools that help improve the quality of my Python code, including flake8, coverage, and mypy. Each of these usually produces some great diagnostics, but none of them are perfect, so they also produce false positives that have to be suppressed. Each of these tools supports "pragmas", i.e. comments you can add to a line of code to suppress a warning. E.g. with flake8 I can say: import readline, rlcompleter # noqa: F401 to suppress the flake8 warnings about unused imports. These modules work by side-effect, which of course a static analyzer can't know. Similarly, when mypy complains about a line it's confused on, I can add a comment to suppress the useless warning: try: from pathlib import Path, PurePath except ImportError: from pathlib2 import Path, PurePath # type: ignore And when I want to boost coverage, but I know that some lines aren't covered under some versions of Python, I can do something like: self.send_error(HTTPStatus.NOT_FOUND) # pragma: nocover These are all well and good until I have to *combine* suppressions. E.g. in the pathlib2 pragma to mypy, I also get a flake8 warning and I've tried just about every combination of pragma comment I can think of, but I've not been able to make both tools happy. I've resorted to refactoring the code into an entire module for flake8 to ignore and added: # flake8: noqa to suppress all warnings in the file. I actually have hit on a few places where I need to suppress warnings from all three tools on the same line, and I basically can't do it. The specifics aren't as important as the general use case: multiple tools competing for the same valuable real-estate. I have no ideas how to improve the situation, and of course any solution would involve some coordination between all of these tools, but it's beginning to feel like a losing battle. Is there a better way? Cheers, -Barry
On 13/11/2017 19:10, Barry Warsaw wrote:
One thing that I have come across with some other toolchains static analysers it to have either or both of: - Suppression comments on a line of their own suppress that specific error on the next non-comment line - Block suppressions that are turned off and then on again. Either of the above could work nicely. -- Steve (Gadget) Barnes Any opinions in this message are my personal opinions and do not reflect those of my employer. --- This email has been checked for viruses by AVG. http://www.avg.com
Off the top of my head there's: - Offer a standard pragma-parser in the standard library's ast module that allows multiple pragmas per line. This is kind of the reason there's the weird ast vs typed_ast discrepancy right now, too. - Create a custom pragma syntax that doesn't use comments. Maybe something like: import stuff @[noqa: 'F401', type: 'ignore'] (Except nicer, because that's ugly as crud.) On Mon, Nov 13, 2017 at 1:10 PM, Barry Warsaw <barry@python.org> wrote:
-- Ryan (ライアン) Yoko Shimomura, ryo (supercell/EGOIST), Hiroyuki Sawano >> everyone else https://refi64.com/
On 2017-11-13 19:10, Barry Warsaw wrote:
I suppose that you could have suggest to them that they follow a convention such as: 1. There can be multiple pragmas in a comment, separated by semicolons: if you don't recognise it, skip past the semicolon. 2. A pragma can be prefixed with the name of the tool, e.g. "# flake8.noqa: F401": if there's a prefix, but it's not yours, skip past the semicolon.
On 13 November 2017 at 20:43, MRAB <python@mrabarnett.plus.com> wrote:
An informational PEP defining a common convention for pragma-style comments could standardise things. I'd suggest starting a discussion (somewhere?) with the development teams for the relevant projects (flake8, mypy, coverage...) with the intention of developing such a PEP that they could all support. Paul
fwiw, we're going to need the tool name in any pragma anyways so the existing thing that should be common is: # tool-name: meaningfultoken It seems like the only convention that makes sense to me. When I saw your flake8 example of "# noqa: F401" I wanted to rip my eyes out. Because it didn't mention the tool name *and* it used a numeric code. Tool authors: use descriptive names! Otherwise it is meaningless to anyone reading it. ex: import rlcompleter # pylint: disable=unused-import Nobody will ever question _what_ that means. -gps On Mon, Nov 13, 2017 at 2:19 PM Brett Cannon <brett@python.org> wrote:
Gregory P. Smith wrote:
One of the things that bother me about end-line comments is that this is going to blow up line length limits. I think this could work if such pragma comments could apply to the following line, and multiline pragmas would be acceptable. Then you could have something like: # flake8: disable=unused-import # mypy: alias=pathlib2.Path # coverage: ignore=when>py2.7
Hah. One of the things I never imagined was that folks would throw around numeric PEP numbers and just expect everyone to have PEP 0 tattooed to the back of their eyelids. Quick, let's discuss PEP 3128! Cheers, -Barry
On Mon, Nov 13, 2017 at 5:37 PM, Barry Warsaw <barry@python.org> wrote:
One of the things that bother me about end-line comments is that this is going to blow up line length limits.
Is that a problem? the linters can ignore lines to long if they are only too long due to one of these :-)
I'd much rather have the tools all look at the entire comment for their particular pragma. and then, even if you wanted to put it on the previous line, you would't have the line-length issue: # flake8: disable=unused-import mypy: alias=pathlib2.Path coverage: ignore=when>py2.7 -CHB -- Christopher Barker, Ph.D. Oceanographer Emergency Response Division NOAA/NOS/OR&R (206) 526-6959 voice 7600 Sand Point Way NE (206) 526-6329 fax Seattle, WA 98115 (206) 526-6317 main reception Chris.Barker@noaa.gov
On Tue, Nov 14, 2017 at 05:12:10PM -0800, Chris Barker wrote:
Yes, it is a problem. See below.
+1 to this.
I'd much rather have the tools all look at the entire comment for their particular pragma.
I have no objection to tooling supporting that as well.
And the irony is that this shows exactly why it is still a problem. Code gets emailed, and lines get wrapped, breaking the too-long line. Even today, we're not entirely paperless, and code gets printed, either trunctating or wrapping too long lines. And of course, code gets read, including comments, and too long lines are, well, too long. -- Steven
On Tue, Nov 14, 2017 at 6:14 PM, Steven D'Aprano <steve@pearwood.info> wrote:
that wrapped a 85 char line (probably to 72) -- lots of folks have set their line length limit to 90+ these days, so I"m not sure we'll evr be safe... but whatever -- multiple line pragmas are fine, too -- I'm hoping putting this much crap in your code will be rare :-) -CHB -- Christopher Barker, Ph.D. Oceanographer Emergency Response Division NOAA/NOS/OR&R (206) 526-6959 voice 7600 Sand Point Way NE (206) 526-6329 fax Seattle, WA 98115 (206) 526-6317 main reception Chris.Barker@noaa.gov
On 16/11/2017 19:03, Chris Barker wrote:
Obviously the best way to avoid multiple warning suppressions to to fix what is being warned about, unfortunately it is not always possible. -- Steve (Gadget) Barnes Any opinions in this message are my personal opinions and do not reflect those of my employer. --- This email has been checked for viruses by AVG. http://www.avg.com
Chris Barker wrote:
but whatever -- multiple line pragmas are fine, too -- I'm hoping putting this much crap in your code will be rare :-)
For sure. It's not uncommon for you to need two pragmas, e.g. flake8 and coverage, but it's pretty rare to need those *plus* mypy. It does happen though. And while I agree it's better to fix the problem, there are lots of reasons why you can't. Multiple-version support in libraries is one common reason (e.g. this code path will never be taken in Python 2 and the alternative will never be taken in Python 3), bugs in the tools (false positives), etc. Cheers, -Barry
I think, if it belongs to a python file, it has to follow the same rules as the rest of the file: - if we follow the pep8, less-than-80 chars remains the rule, whether we are in a comment or not - if the first of 3 following codes works, all three should (the 3 comments refer to the line of code): some_function(foo, bar) # coverage: ignore=when>py2.7, alias=pathlib2.Path # coverage: ignore=when>py2.7, alias=pathlib2.Path some_function(foo, bar) # coverage: ignore=when>py2.7 # alias=pathlib2.Path some_function(foo, bar) Also, having these pragmas instructions should not prevent classic commenting in any ways, or make them harder to write or understand, so those should all be valid: # doing something important here some_function(foo, bar) # coverage: ignore=when>py2.7, alias=pathlib2.Path # coverage: ignore=when>py2.7, alias=pathlib2.Path some_function(foo, bar)# doing something important here # coverage: ignore=when>py2.7 # alias=pathlib2.Path # doing something important here some_function(foo, bar) or (what's better for readability?) # doing something important here # coverage: ignore=when>py2.7 # alias=pathlib2.Path some_function(foo, bar) Also, why not adopt the same syntax that is already quite commonly used for todos in many languages, with "@"? # @todo: write tests for this function! @coverage: ignore=when>py2.7 @alias=pathlib2.Path some_function(foo, bar) -Brice Le 15/11/17 à 02:12, Chris Barker a écrit :
On Mon, Nov 13, 2017 at 06:37:05PM -0500, Barry Warsaw wrote:
Brett may have meant https://mail.python.org/mailman/listinfo/code-quality -- Steve
On Mon, Nov 13, 2017, 15:55 Steven D'Aprano, <steve@pearwood.info> wrote:
Steve's right. http://meta.pycqa.org/en/latest/ -brett
On 14 November 2017 at 10:27, Brett Cannon <brett@python.org> wrote:
I think that will be the right way to go about it, as this seems similar to a lot of the problems that arise in the packaging interoperability space: what really matters is what tool developers support in practice, so documenting a proposed convention without a clear implementation plan across common tools wouldn't really achieve a great deal. This means that any new convention ideas need to be discussed and supported by the tool developers, with python-dev & python-ideas really only getting involved if some standard library adjustments are needed (e.g. updating doctest to abide by any proposed conventions), or if the convention is going to be captured as an informational PEP, rather than solely as a recommended comment parsing library. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Steven D'Aprano wrote:
Brett may have meant
That definitely makes more sense. :) -Barry
On 11/13/2017 4:58 PM, Paul Moore wrote:
This seems like a good subject for an informational PEP. It could and I think should include an implementation of the convention as a generator function with parameter 'tool' that yields (option, value) pairs for that tool. Docstring directives might be considered for points to copy or not copy. An example is # doctest: +ELLIPSIS, +NORMALIZE_WHITESPACE '.' instead of ': ' to separate the tool prefix seems better for multiple tools, except that the latter works better for omitting the tool prefix when there are multiple options. For binary options, '+' and '-' prefixes are much more compact than '=True' and '=False' suffixes. This will be more important with multiple directives in one comment. -- Terry Jan Reedy
On 11/13/17 2:10 PM, Barry Warsaw wrote:
Coverage.py has no fixed syntax for its pragmas. The default is "# pragma: no cover", but you can configure any regex you like, and it's matched against the entire line, not just the comment. This makes it possible to exclude lines based on the source, or to change the syntax of the pragma. If a PEP is written with a common syntax, I can add it to coverage.py's default. --Ned.
On 13/11/2017 19:10, Barry Warsaw wrote:
One thing that I have come across with some other toolchains static analysers it to have either or both of: - Suppression comments on a line of their own suppress that specific error on the next non-comment line - Block suppressions that are turned off and then on again. Either of the above could work nicely. -- Steve (Gadget) Barnes Any opinions in this message are my personal opinions and do not reflect those of my employer. --- This email has been checked for viruses by AVG. http://www.avg.com
Off the top of my head there's: - Offer a standard pragma-parser in the standard library's ast module that allows multiple pragmas per line. This is kind of the reason there's the weird ast vs typed_ast discrepancy right now, too. - Create a custom pragma syntax that doesn't use comments. Maybe something like: import stuff @[noqa: 'F401', type: 'ignore'] (Except nicer, because that's ugly as crud.) On Mon, Nov 13, 2017 at 1:10 PM, Barry Warsaw <barry@python.org> wrote:
-- Ryan (ライアン) Yoko Shimomura, ryo (supercell/EGOIST), Hiroyuki Sawano >> everyone else https://refi64.com/
On 2017-11-13 19:10, Barry Warsaw wrote:
I suppose that you could have suggest to them that they follow a convention such as: 1. There can be multiple pragmas in a comment, separated by semicolons: if you don't recognise it, skip past the semicolon. 2. A pragma can be prefixed with the name of the tool, e.g. "# flake8.noqa: F401": if there's a prefix, but it's not yours, skip past the semicolon.
On 13 November 2017 at 20:43, MRAB <python@mrabarnett.plus.com> wrote:
An informational PEP defining a common convention for pragma-style comments could standardise things. I'd suggest starting a discussion (somewhere?) with the development teams for the relevant projects (flake8, mypy, coverage...) with the intention of developing such a PEP that they could all support. Paul
fwiw, we're going to need the tool name in any pragma anyways so the existing thing that should be common is: # tool-name: meaningfultoken It seems like the only convention that makes sense to me. When I saw your flake8 example of "# noqa: F401" I wanted to rip my eyes out. Because it didn't mention the tool name *and* it used a numeric code. Tool authors: use descriptive names! Otherwise it is meaningless to anyone reading it. ex: import rlcompleter # pylint: disable=unused-import Nobody will ever question _what_ that means. -gps On Mon, Nov 13, 2017 at 2:19 PM Brett Cannon <brett@python.org> wrote:
Gregory P. Smith wrote:
One of the things that bother me about end-line comments is that this is going to blow up line length limits. I think this could work if such pragma comments could apply to the following line, and multiline pragmas would be acceptable. Then you could have something like: # flake8: disable=unused-import # mypy: alias=pathlib2.Path # coverage: ignore=when>py2.7
Hah. One of the things I never imagined was that folks would throw around numeric PEP numbers and just expect everyone to have PEP 0 tattooed to the back of their eyelids. Quick, let's discuss PEP 3128! Cheers, -Barry
On Mon, Nov 13, 2017 at 5:37 PM, Barry Warsaw <barry@python.org> wrote:
One of the things that bother me about end-line comments is that this is going to blow up line length limits.
Is that a problem? the linters can ignore lines to long if they are only too long due to one of these :-)
I'd much rather have the tools all look at the entire comment for their particular pragma. and then, even if you wanted to put it on the previous line, you would't have the line-length issue: # flake8: disable=unused-import mypy: alias=pathlib2.Path coverage: ignore=when>py2.7 -CHB -- Christopher Barker, Ph.D. Oceanographer Emergency Response Division NOAA/NOS/OR&R (206) 526-6959 voice 7600 Sand Point Way NE (206) 526-6329 fax Seattle, WA 98115 (206) 526-6317 main reception Chris.Barker@noaa.gov
On Tue, Nov 14, 2017 at 05:12:10PM -0800, Chris Barker wrote:
Yes, it is a problem. See below.
+1 to this.
I'd much rather have the tools all look at the entire comment for their particular pragma.
I have no objection to tooling supporting that as well.
And the irony is that this shows exactly why it is still a problem. Code gets emailed, and lines get wrapped, breaking the too-long line. Even today, we're not entirely paperless, and code gets printed, either trunctating or wrapping too long lines. And of course, code gets read, including comments, and too long lines are, well, too long. -- Steven
On Tue, Nov 14, 2017 at 6:14 PM, Steven D'Aprano <steve@pearwood.info> wrote:
that wrapped a 85 char line (probably to 72) -- lots of folks have set their line length limit to 90+ these days, so I"m not sure we'll evr be safe... but whatever -- multiple line pragmas are fine, too -- I'm hoping putting this much crap in your code will be rare :-) -CHB -- Christopher Barker, Ph.D. Oceanographer Emergency Response Division NOAA/NOS/OR&R (206) 526-6959 voice 7600 Sand Point Way NE (206) 526-6329 fax Seattle, WA 98115 (206) 526-6317 main reception Chris.Barker@noaa.gov
On 16/11/2017 19:03, Chris Barker wrote:
Obviously the best way to avoid multiple warning suppressions to to fix what is being warned about, unfortunately it is not always possible. -- Steve (Gadget) Barnes Any opinions in this message are my personal opinions and do not reflect those of my employer. --- This email has been checked for viruses by AVG. http://www.avg.com
Chris Barker wrote:
but whatever -- multiple line pragmas are fine, too -- I'm hoping putting this much crap in your code will be rare :-)
For sure. It's not uncommon for you to need two pragmas, e.g. flake8 and coverage, but it's pretty rare to need those *plus* mypy. It does happen though. And while I agree it's better to fix the problem, there are lots of reasons why you can't. Multiple-version support in libraries is one common reason (e.g. this code path will never be taken in Python 2 and the alternative will never be taken in Python 3), bugs in the tools (false positives), etc. Cheers, -Barry
I think, if it belongs to a python file, it has to follow the same rules as the rest of the file: - if we follow the pep8, less-than-80 chars remains the rule, whether we are in a comment or not - if the first of 3 following codes works, all three should (the 3 comments refer to the line of code): some_function(foo, bar) # coverage: ignore=when>py2.7, alias=pathlib2.Path # coverage: ignore=when>py2.7, alias=pathlib2.Path some_function(foo, bar) # coverage: ignore=when>py2.7 # alias=pathlib2.Path some_function(foo, bar) Also, having these pragmas instructions should not prevent classic commenting in any ways, or make them harder to write or understand, so those should all be valid: # doing something important here some_function(foo, bar) # coverage: ignore=when>py2.7, alias=pathlib2.Path # coverage: ignore=when>py2.7, alias=pathlib2.Path some_function(foo, bar)# doing something important here # coverage: ignore=when>py2.7 # alias=pathlib2.Path # doing something important here some_function(foo, bar) or (what's better for readability?) # doing something important here # coverage: ignore=when>py2.7 # alias=pathlib2.Path some_function(foo, bar) Also, why not adopt the same syntax that is already quite commonly used for todos in many languages, with "@"? # @todo: write tests for this function! @coverage: ignore=when>py2.7 @alias=pathlib2.Path some_function(foo, bar) -Brice Le 15/11/17 à 02:12, Chris Barker a écrit :
On Mon, Nov 13, 2017 at 06:37:05PM -0500, Barry Warsaw wrote:
Brett may have meant https://mail.python.org/mailman/listinfo/code-quality -- Steve
On Mon, Nov 13, 2017, 15:55 Steven D'Aprano, <steve@pearwood.info> wrote:
Steve's right. http://meta.pycqa.org/en/latest/ -brett
On 14 November 2017 at 10:27, Brett Cannon <brett@python.org> wrote:
I think that will be the right way to go about it, as this seems similar to a lot of the problems that arise in the packaging interoperability space: what really matters is what tool developers support in practice, so documenting a proposed convention without a clear implementation plan across common tools wouldn't really achieve a great deal. This means that any new convention ideas need to be discussed and supported by the tool developers, with python-dev & python-ideas really only getting involved if some standard library adjustments are needed (e.g. updating doctest to abide by any proposed conventions), or if the convention is going to be captured as an informational PEP, rather than solely as a recommended comment parsing library. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Steven D'Aprano wrote:
Brett may have meant
That definitely makes more sense. :) -Barry
On 11/13/2017 4:58 PM, Paul Moore wrote:
This seems like a good subject for an informational PEP. It could and I think should include an implementation of the convention as a generator function with parameter 'tool' that yields (option, value) pairs for that tool. Docstring directives might be considered for points to copy or not copy. An example is # doctest: +ELLIPSIS, +NORMALIZE_WHITESPACE '.' instead of ': ' to separate the tool prefix seems better for multiple tools, except that the latter works better for omitting the tool prefix when there are multiple options. For binary options, '+' and '-' prefixes are much more compact than '=True' and '=False' suffixes. This will be more important with multiple directives in one comment. -- Terry Jan Reedy
On 11/13/17 2:10 PM, Barry Warsaw wrote:
Coverage.py has no fixed syntax for its pragmas. The default is "# pragma: no cover", but you can configure any regex you like, and it's matched against the entire line, not just the comment. This makes it possible to exclude lines based on the source, or to change the syntax of the pragma. If a PEP is written with a common syntax, I can add it to coverage.py's default. --Ned.
participants (13)
-
Barry Warsaw -
Brett Cannon -
Brice Parent -
Chris Barker -
Gregory P. Smith -
MRAB -
Ned Batchelder -
Nick Coghlan -
Paul Moore -
Ryan Gonzalez -
Steve Barnes -
Steven D'Aprano -
Terry Reedy