work around awkward situation with float subclasses #528
Conversation
Summary:
On python before 3.9, `float` is a subclass of the ABC numbers.Real which
defines abstract methods `__floor__` and `__ceil__`, but doesn't provide
an actual implementation of those methods. This is arguably a type error
in Python, because the following code is well-typed:
def foo(x: numbers.Real): return x.__floor()
def bar(x: float): return foo(x)
bar(some float)
but fail at runtime. This doesn't create a user-visible problem because
`__floor__` and `__ceil__` aren't meant to be called directly; instead,
users are supposed to call `math.floor(x)` and `math.ceil(x)` which are
fine without the dunder methods as long as the object is a primitive
number (or a subclass of one, or can be converted to one in various
ways.)
This creates the unfortunate situation that Pyre won't let people
instantiate subclasses of `float` without definitions of `__floor__` and
`__ceil__`, even though no typical code actually requires those methods.
(Pyre currently has a special case to allow the instantiation of `float`
itself.)
The ideal behavior would be if Pyre treated `numbers.Real` as "in order
to be a subtype of `numbers.Real`, you need to implement the floor/ceil
dunder methods, or be a subtype of int/float/etc, or implement
`__float__` or `__index__`, etc." and "numbers.Real are not guaranteed
to define `__floor__` and `__ceil__`. This would be insane to implement.
Pretending that `float` defines `__floor__` and `__ceil__` is a
minimally objectionable workaround. It means that float will continue to
work, subclasses of float will start to work, and the only well-typed
code which can error at runtime is `val.__floor__()` when
`type(val) == T <: float`... But that problem already exists when
`T == float` and so I don't find extending it to subclasses of float to
be particularly concerning.
Resolves facebook#527 (github)
Test plan:
These two examples fail to typecheck before:
# example 1
import typing
T = typing.NewType('T', float)
T(4.0)
# example 2
class C(float):
def __init__(self, v: float) -> None: pass
C(4.0)
with errors:
example.py:3:0 Invalid class instantiation [45]:
Cannot instantiate abstract class `T` with abstract methods `__ceil__`, `__floor__`.
example.py:9:14 Invalid class instantiation [45]:
Cannot instantiate abstract class `C` with abstract methods `__ceil__`, `__floor__`.
but both pass after.
|
Hi @jesboat! Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention. You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@arthaud has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
Out of curiosity, did the Summary/Test Plan in my commit message end up in the right phabricator fields? |
|
Yes it did :) |
|
Question from @grievejia: could we fix this in typeshed directly? |
Summary:
On python before 3.9,
floatis a subclass of the ABC numbers.Real whichdefines abstract methods
__floor__and__ceil__, but doesn't providean actual implementation of those methods. This is arguably a type error
in Python, because the following code is well-typed:
but fail at runtime. This doesn't create a user-visible problem because
__floor__and__ceil__aren't meant to be called directly; instead,users are supposed to call
math.floor(x)andmath.ceil(x)which arefine without the dunder methods as long as the object is a primitive
number (or a subclass of one, or can be converted to one in various
ways.)
This creates the unfortunate situation that Pyre won't let people
instantiate subclasses of
floatwithout definitions of__floor__and__ceil__, even though no typical code actually requires those methods.(Pyre currently has a special case to allow the instantiation of
floatitself.)
The ideal behavior would be if Pyre treated
numbers.Realas "in orderto be a subtype of
numbers.Real, you need to implement the floor/ceildunder methods, or be a subtype of int/float/etc, or implement
__float__or__index__, etc." and "numbers.Real are not guaranteedto define
__floor__and__ceil__. This would be insane to implement.Pretending that
floatdefines__floor__and__ceil__is aminimally objectionable workaround. It means that float will continue to
work, subclasses of float will start to work, and the only well-typed
code which can error at runtime is
val.__floor__()whentype(val) == T <: float... But that problem already exists whenT == floatand so I don't find extending it to subclasses of float tobe particularly concerning.
Resolves #527 (github)
Test plan:
These two examples fail to typecheck before:
with errors:
but both pass after.