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

Should PYI041 consider bool to be redundant with int? #9810

Open
tylerlaprade opened this issue Feb 3, 2024 · 5 comments
Open

Should PYI041 consider bool to be redundant with int? #9810

tylerlaprade opened this issue Feb 3, 2024 · 5 comments
Assignees
Labels
question Asking for support or clarification

Comments

@tylerlaprade
Copy link
Contributor

Currently, this rule only flags int | float unions as redundant. Coming from other languages, I didn't even realize int was a subtype of float, but I learned about it because of this rule. However, I found a bug in my code where I was converting False to 0.0 - this was because in Python, bool is actually an int. I found that the tests for this rule very explicitly consider int | bool to not be a violation, however this would have prevented me from writing the bug as I did.

image

The bool to int relation is in some ways stronger than int to float since isinstance() only recognizes the former:
image

@charliermarsh
Copy link
Member

@AlexWaygood - I defer to you on this one.

@charliermarsh charliermarsh added the question Asking for support or clarification label Feb 5, 2024
@AlexWaygood AlexWaygood self-assigned this Feb 5, 2024
@zanieb
Copy link
Member

zanieb commented Feb 5, 2024

I don't think this makes sense to me, they seem more distinct than int / float regardless of the isinstance behavior. I'm curious to hear more opinions though!

@tylerlaprade
Copy link
Contributor Author

To me, they're very different in intended usage, but surprisingly related in actual effect, which threw me off, especially since the rule warned about the other scenario and not this one.

@maxyz
Copy link

maxyz commented Mar 15, 2024

I just stumbled into this rule, and I think the rule is wrong.

The PEP mentioned in the rule documentation ( PEP 3141 ) is talking about the numbers ABCs, these are, numbers.Complex, numbers.Real and numbers.Integral.

float is a particular class that implements numbers.Real, int is a particular class that implements. But they don't have a relationship between them.

Afaik, the numerical hierarchy defined in the pep-3141 can be seen/tested using:

import numbers

i = 1
f = 1.0

isinstance(i, int) # True
isinstance(i, float) # False

isinstance(f, int) # False
isinstance(f, float) # True

isinstance(i, numbers.Complex) # True
isinstance(i, numbers.Real) # True
isinstance(i, numbers.Integral) # True

isinstance(f, numbers.Complex) # True
isinstance(f, numbers.Real) # True
isinstance(f, numbers.Integral) # False

Am I missing something?

@AlexWaygood
Copy link
Member

I just stumbled into this rule, and I think the rule is wrong.

The PEP mentioned in the rule documentation ( PEP 3141 ) is talking about the numbers ABCs, these are, numbers.Complex, numbers.Real and numbers.Integral.

The rule isn't wrong @maxyz, but you're right that it's quite misleading for the docs to mention PEP 3141. The rule is premised on the fact that type checkers do not support PEP 3141 (and never will) -- the ABCs introduced by that PEP are wholly incompatible with Python's static-typing system and impossible to annotate in a desirable way in typeshed.

For more, see:

The "numeric tower" that the docs should be linking to is this one here: https://peps.python.org/pep-0484/#the-numeric-tower. (If you think this situation is confusing, you're not alone :)

I'll make a docs PR to improve this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Asking for support or clarification
Projects
None yet
Development

No branches or pull requests

5 participants