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

ANN001 should have an option to not fire when a non-None default argument is provided #5958

Open
tylerlaprade opened this issue Jul 21, 2023 · 8 comments
Labels
needs-decision Awaiting a decision from a maintainer

Comments

@tylerlaprade
Copy link
Contributor

This was already raised in #794 but closed for unclear reasons. It seemed to have positive support in that thread. When I do something like

def make_test_foo(
    bar: Bar | None = None,
    create_bar=False,
    baz=None,
    create_baz=True,
    qux: Qux | None = None,
    create_qux=True,
):

in my test fixtures, I want my attention to be called to the missing type annotation on baz, not unnecessary warnings on create_bar, create_baz, and create_qux, because Pyright is already safely assuming them to be bool types.

This could be implemented as either breaking up ANN001 into two separate rules, or as an additional config option as mentioned in #794.

I personally think the former option results in a cleaner settings file, but it would obviously be a somewhat breaking change unless the new rule was added as ANN0011 so that people would have to opt out of it (I don't believe any such codes currently exist, though, so it would be a deviation from the norm). The best option is therefore likely to be a new config flag, although I am happy to see this implemented in any form possible!

@tylerlaprade tylerlaprade changed the title ANN001 should have an option to not fire when default argument is provided ANN001 should have an option to not fire when a non-None default argument is provided Jul 21, 2023
@charliermarsh charliermarsh added the needs-decision Awaiting a decision from a maintainer label Jul 23, 2023
@charliermarsh
Copy link
Member

I suspect it was closed because Mypy infers those types as Any (e.g., for create_baz).

@tylerlaprade
Copy link
Contributor Author

Ah, I see. Given that Pyright (and, I assume, Pytype and Pyre, although I haven't used them) behave(s) differently, I'd like to keep the discussion open.

@tylerlaprade
Copy link
Contributor Author

This is by far my #1 most desired feature at the moment because, without it, I don't feel comfortable forcing this rule upon my teammates. Is there any information I can provide to help reach a decision?

@zanieb
Copy link
Member

zanieb commented Jul 26, 2023

@tylerlaprade Is there an issue on mypy and/or pyright discussing this difference in behavior?

@zanieb
Copy link
Member

zanieb commented Jul 26, 2023

Also, in your use-case would this apply to other types than bool?

@tylerlaprade
Copy link
Contributor Author

Also, in your use-case would this apply to other types than bool?

Ideally, it would apply to all non-None types. Pyright assumes a type for all of them.

Is there an issue on mypy and/or pyright discussing this difference in behavior?

I can go take a look, but I'm not sure why it would be relevant here. It's definitely not an unintentional feature of Pyright.

@zanieb
Copy link
Member

zanieb commented Jul 26, 2023

Per the discussion at python/mypy#3090 (comment) PEP-484 says the inferred type should be Any (https://peps.python.org/pep-0484/#the-any-type)

It'd be great to get that amended on the language standard and have some consensus around inference. It seems like that's probably a ways off but perhaps we can start a discussion about that in the relevant forum. In the meantime, this does feel reasonable to implement although I'm not sure of the best way how.

@tylerlaprade
Copy link
Contributor Author

tylerlaprade commented Jul 27, 2023

I don't know Rust (yet), otherwise I'd be happy to attempt it myself. There are other existing type-aware rules, right? Is there anything they do that can be reused?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer
Projects
None yet
Development

No branches or pull requests

3 participants