black
black copied to clipboard
Behavior of `# fmt: skip` in the presence of other comments/pragmas
I have this code:
def function():
from . import something_long_long_long_long # pylint: disable=import-outside-toplevel
black formats it to:
def function():
from . import (
something_long_long_long_long,
) # pylint: disable=import-outside-toplevel
This breaks the Pylint pragma, it does not work anymore on the line where black has put it. So, I played around with the pylint and black pragmas to get it working:
def function():
# option 1: move the pylint pragma after black did its work
from . import ( # pylint: disable=import-outside-toplevel
something_long_long_long_long,
)
# option 2: try fmt: skip at the end of the line
from . import something_long_long_long_long # pylint: disable=import-outside-toplevel # fmt: skip
# option 3: try fmt: skip at the beginning of the comment
from . import something_long_long_long_long # fmt: skip # pylint: disable=import-outside-toplevel
# option 4: use the off/on pragmas
# fmt: off
from . import something_long_long_long_long # pylint: disable=import-outside-toplevel
# fmt: on
All those 4 options are fine for Pylint. Options 1 and 4 are fine for Black. Black ignores the # fmt: skip pragmas in options 2 & 3.
Option 4 is my current workaround, though it's not quite nice having to use two pragmas on additional lines.
In my opinion, black should at least work with the skip pragmas as in option 2 (at the end of the line). Maybe also option 3 (the first part of the comment). In the current documentation there is this wording:
[Black] doesn’t reformat lines that end with
# fmt: skip
This suggests that option 2 should work, but it doesn't. The documentation should be made clearer, especially if this issue would not be fixed.
An idea is also to exclude Pylint pragmas (and possibly pragmas from other tools) from calculating the line length in black. Pylint does this with its own pragmas (there is also a Pylint line length check), which makes sense because pragmas are usually not something that one needs to read very often, hence it's kinda acceptable to exceed the line limit.
(Related to https://github.com/psf/black/issues/3329, which is a matter of style, this here is about interoperability with other tools.)
I have here another example which illustrates that using the off/on pragmas ("option 4" in my original posting) has drawbacks in certain situations:
class A:
def method(self):
# fmt: off
for value in self.plugin.job_parameters._data.values(): # pylint: disable=protected-access
# code here would not be formatted by black
...
# fmt: on
# formatting can be turned on again earliest after the whole for-loop block
An alternative here could be to refactor further, this could make the use of # fmt: off/on obsolete, or at least make it possible to use it around a sole statement, and not around a whole block:
class A:
def method(self):
# fmt: off
workaround_name_might_be_long_too = self.something_long._data # pylint: disable=protected-access
# fmt: on
for value in workaround_name_might_be_long_too.values():
# this block now gets properly formatted
This shall illustrate that it is useful to have a single-line pragma (for whatever tool), as well as that it would be useful if those pragmas could be used with pragmas of other tools on the same line. Otherwise, it would lead here and there to annoying workarounds.
What do you think about this: The # fmt: skip pragma should also work if it is embedded in the comment, but properly separated with # and two spaces. E.g.
statement # something before # fmt: skip
statement # fmt: skip # something after
statement # before # fmt: skip # after
Edit: I think there is the common approach to separate different pragmas via a semicolon, so, what about checking whether any partition of the comment - divided either by ; or #, equals fmt: skip?
Related: https://github.com/psf/black/issues/2843#issuecomment-1232146924 where I proposed a few options related to how Black can statistically improve pragma comments related line wrappings.
I have another interesting example. It is about and odd behavior between black, isort, and pylint.
There is this code, which is fine for isort and also Pylint. The Pylint pragma works, and isort does not reformat the statement - which apparently means that it does not take the Plyint pragma for line length into account:
from my_plugins import abc_plugin as abc_plugin_with_odd_name # pylint: disable=unused-import
But for black the line is too long, and it formats it into:
from my_plugins import (
abc_plugin as abc_plugin_with_odd_name,
) # pylint: disable=unused-import
This breaks Pylint, because the pragma does not work anymore on the line where it was put. Running isort again will revert the thing back to the original code. The only way out is some user interaction. Let's put the Pylint pragma where it would work:
from my_plugins import ( # pylint: disable=unused-import
abc_plugin as abc_plugin_with_odd_name,
)
This works now for Pylint and for black! But not for isort... it would reformat it again back to the original code (into a single line). Let's try to add also a pragma for isort:
from my_plugins import ( # pylint: disable=unused-import # isort:skip
abc_plugin as abc_plugin_with_odd_name,
)
This works now for black, isort, and Pylint. (It does not matter if the two pragmas are separated with a # or a ;.)
By the way, in this case it would not be possible to use black's fmt: off/on multiline pragmas (outlined in my earlier postings as option 4), because isort would put those comments at different places, and would not keep it exactly around the one import statement.
Implementing option 2 (the single line fmt: skip pragma, see above) would improve the situation, one would be able to find a solution easier than with playing around where to put which pragma of which tool.. ~~In my opinion, this would still only be a workaround - if isort supports ignoring Pylint pragmas for the line length, and black is apparently interested in smooth tool interoperability (see the docs!), then I think also black should ignore pragmas for the line length.~~ Edit: I think this changed somewhere from isort 4.3.21 to 5.10.1.. in v5 also isort does break overlong pylint pragmas - which is what black does. I did not find a related isort issue on Github.
If you need any more information, please just ask for it.
I was using
black 22.10.0 (default config)
isort 5.10.1 (with force_single_line=True and the compat. options)
pylint 2.14.3
This issue goes into a bit of extraneous detail, but I think there's a fairly clear bug. Given this code:
x+1 # fmt: skip
x+2 # fmt: skip # hello
x+3 # hello # fmt: skip
Black will currently leave 1 alone, but format 2 and 3 (adding spaces around the +). I agree that at least one of 2 and 3 should be left alone, probably both. I would accept a PR making that change.
This issue goes into a bit of extraneous detail, but I think there's a fairly clear bug. Given this code:
x+1 # fmt: skip x+2 # fmt: skip # hello x+3 # hello # fmt: skipBlack will currently leave 1 alone, but format 2 and 3 (adding spaces around the
+). I agree that at least one of 2 and 3 should be left alone, probably both. I would accept a PR making that change.
2 means why we are skipping formatting this line. 3 means we must skip formatting this line, because the comment explaining the code breaks formatting.
Both should honor the directive, IMHO.
x+1 # fmt: skip x+2 # fmt: skip # hello x+3 # hello # fmt: skipBlack will currently leave 1 alone, but format 2 and 3 (adding spaces around the
+). I agree that at least one of 2 and 3 should be left alone, probably both. I would accept a PR making that change.
From reading around (and if interpreting correctly), it looks like an extreme version of case 3 was not accepted in https://github.com/psf/black/issues/3450.
To share an opinion for case 2, I am a fan of allowing one to document reasoning on the same line (less overall lines, reasoning is closest to disable):
x+2 # fmt: skip # Some reasoning for skipping
# Or
# fmt: off # Some reasoning for turning off
x+2
# fmt: on # Some reasoning for turning on
I think @ricardo-dematos had the same opinion on case 2 in his comment above. Would be cool to allow this
#3450 turned out to be a different request to ignore comments for the purpose of line length, it's not relevant here as this issue is only about # fmt: skip.
I think we should leave both cases (2) and (3) from my message alone, which is also what @ricardo-dematos argued for above.
If you're interested in seeing this behavior in Black, please send a pull request to implement it.
I wanted to silence pylint and prevent from formatting this line:
import pdb; pdb.set_trace() # fmt: skip # pylint disable=all
As mentioned mixing the pragmas doesn't work however in other issues I've found the following workaround. Both pylint and black ignore this line.
__import__("pdb").set_trace()
Duplicate of #2213