CLN: small cleanups #48836
CLN: small cleanups #48836
Conversation
| ... | ||
|
|
||
|
|
||
| def validate_periods(periods: float | None) -> int | None: | ||
| def validate_periods(periods: int | float | None) -> int | None: |
There was a problem hiding this comment.
@twoertwein so the typing simplification(?) that converted int | float -> float isn't an enforced typing check?
There was a problem hiding this comment.
flake8-pyi does that check but only for pyi files not py files.
More generally, we have many Unions that contain redundant classes, for example Scalar contains datetime and pd.Timestamp but I think pd.Timestamp inherits from datetime. That is not a problem (also type checkers might need some more time) but it is not good style.
There was a problem hiding this comment.
Should I revert this? i find it clearer, but not a big deal.
There was a problem hiding this comment.
I wouldn't mind ignoring flake8-pyi's Y041 when we run flake8-pyi: then py and pyi files are more consistent and there is also some push back against it python/mypy#13760 (comment)
There was a problem hiding this comment.
btw in a world where float is equivalent to int | float, how would i express "specifically a python float object and not an integer object"
There was a problem hiding this comment.
In the type checkers world, you can't: https://peps.python.org/pep-0484/#the-numeric-tower
Rather than requiring that users write import numbers and then use numbers.Float etc., this PEP proposes a straightforward shortcut that is almost as effective: when an argument is annotated as having type float, an argument of type int is acceptable; similar, for an argument annotated as having type complex, arguments of type float or int are acceptable. This does not handle classes implementing the corresponding ABCs or the fractions.Fraction class, but we believe those use cases are exceedingly rare.
But I wonder how python and its type system would have evolved if they started together - we hopefully wouldn't have this mess of type checkers/PEPs dictating that int is treated as a subclass of float but the implementation doesn't do that.
There was a problem hiding this comment.
that seems weird given that int isnt a subclass of float. anyway, shouldnt be relevant for this PR
mroeschke
added
Clean
Non-Nano
|
Thanks @jbrockmendel |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.