single-element tuple

[JelleZijlstra]

No tests yet, just want to show what this does on typeshed

Fixes #131

[JelleZijlstra]

False positives included:

• A number of __getnewargs__ overrides, which often return a single-element tuple
• Some cases where we are very precise about a tuple with at most two elements
• Some exceptions that store a single argument in an args tuple

[AlexWaygood]

Okay, you've persuaded me that this is a serious problem that needs some kind of automated check. Let's do it, but figure out how we can limit the false positives.

Seems like __getnewargs__ methods and args attributes are common causes of false-positives, so let's just not raise the error for those specific situations, and raise it in all other situations.

[JelleZijlstra]

I had kind of the opposite reaction :) It's a lot of false positives.

I feel like my ideal state would be a non-blocking GitHub Action that adds a comment like "did you really want a single-element tuple here?".

[AlexWaygood]

I feel like my ideal state would be a non-blocking GitHub Action that adds a comment like "did you really want a single-element tuple here?".

Clippy but for code. Sure.

[Akuli]

Another alternative would be to add some heuristics when the warning won't trigger. Starting with the 52 false positives that remain, we can get to:

• 32 false positives, if we don't warn about __getnewargs__ and __reduce__ methods
• 29 false positives, if we also skip __new__ (if you're doing something with __new__, that's somewhat advanced anyway and will be reviewed carefully)
• 24 false positives, if we skip classes whose name contains Error or Exception, or have a base class whose name contains Error or Exception
• 18 false positives, if we ignore methods that have an argument whose name ends with 1, such as __iter1
• 15 false positives, if we ignore tuple[Foo] in unions that also contain tuple[Foo, Bar]

(These numbers could be off by one, because they were generated by an ugly script.)

In practice, implementing all of these checks is overkill, and just skipping __getnewargs__, __reduce__ and __new__ might be good enough.

[Akuli]

I think I found the combination that reduces the most false positives compared to how much work it would be to implement:

• Ignore __getnewargs__ and __reduce__ methods
• Ignore methods that have an argument name ending with 1

Just these two bring us from 52 false positives to 21 false positives.

[JelleZijlstra]

Ignore methods that have an argument name ending with 1

I don't like that because it's going to be very unintuitive for users. I could get behind excluding the pickle-related methods though.

[JelleZijlstra]

Also @Akuli would you mind double checking the two hits in tkinter? It wasn't really clear from the code what was going on there.

./stdlib/tkinter/ttk.pyi:997:79: Y032 Single-element tuple
./stdlib/tkinter/ttk.pyi:1037:60: Y032 Single-element tuple

[Akuli]

Line 997: The annotations have a lot of room for improvement. But tuple[str] is "correct", in the sense that it is one of the possible types. A more correct annotation would apparently be str | tuple[str]. But specifying multiple images doesn't make sense.

>>> import tkinter.ttk
>>> tv = tkinter.ttk.Treeview()
>>> image = tkinter.PhotoImage(file="/usr/share/icons/hicolor/16x16/audacity.png")
>>> tv.heading('#0', 'image')
''
>>> tv.heading('#0', image=image)
{}
>>> tv.heading('#0', 'image')
'pyimage1'
>>> tv.pack()
>>> tv.heading('#0', 'image')
('pyimage1',)
('pyimage1',)

The annotation of item has a similar situation, although it seems that Literal[""] | tuple[str] might work for that.

[sobolevn]

I am +1 on non-blocking GitHub Action suggestion. The same for ParamSpec.
These two are not rules, but observations.

We don't want to turn our code base into noqa filled one 🙂

[sobolevn]

Implementation specific, we can add a new code for it, like OPY00n (like in OPtional Y, where n is an issue number).
And disable it by default (supported by flake8). So, all our users will have the same exact features as they do now.

But, later we can enable it in our CI like flake8 --enable-extensions OPY --select OPY with something like reviewdog integration: https://github.com/wemake-services/wemake-python-styleguide/blob/61d4fa55c7898bb124ccf573219e8ccaac543742/scripts/entrypoint.sh#L18-L22

So, our users will be warned with minimal damage done! 👍

And, please, don't forget about non-typeshed users like me 🙂
https://github.com/dry-python/returns/blob/master/pyproject.toml#L78

[AlexWaygood]

And disable it by default (supported by flake8)

We actually had some trouble getting disabled-by-default errors to work ☹️

• Compatibility with flake8 --extend-select #75
• Revisit disabled-by-default errors #86

Do you know what we might have been doing wrong/how to get it working?

[sobolevn]

I have some plugins that use explicit https://flake8.pycqa.org/en/latest/user/options.html?highlight=enable-extensions#cmdoption-flake8-enable-extensions with no problems.

Example: https://github.com/globality-corp/flake8-logging-format#violations-detected