-
Notifications
You must be signed in to change notification settings - Fork 85
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
Improve Int and Float type stubs #1656
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good. A couple of questions.
@@ -11,6 +11,18 @@ | |||
from traits.api import Float, HasTraits | |||
|
|||
|
|||
class HasIndex: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dumb question (and probably not for this PR in any case): do we have a test for this sort of thing in traits test suite?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm fairly sure we do. :-) I'll check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Int:
traits/traits/tests/test_integer.py
Lines 32 to 34 in 80294cf
class IntegerLike(object): | |
def __index__(self): | |
return 42 |
Float:
traits/traits/tests/test_float.py
Lines 21 to 44 in 80294cf
class IntegerLike: | |
def __init__(self, value): | |
self._value = value | |
def __index__(self): | |
return self._value | |
# Python versions < 3.8 don't support conversion of something with __index__ | |
# to float. | |
try: | |
float(IntegerLike(3)) | |
except TypeError: | |
float_accepts_index = False | |
else: | |
float_accepts_index = True | |
class MyFloat(object): | |
def __init__(self, value): | |
self._value = value | |
def __float__(self): | |
return self._value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. which reminds me that SupportsIndex
isn't actually quite right for the Float
trait type with Python < 3.8. I don't think I care enough to try to distinguish. For the use-cases we care about (e.g., NumPy int types), those types implement __float__
anyway.
@@ -57,7 +66,7 @@ class _BaseInt(_TraitType[_T, int]): | |||
... | |||
|
|||
|
|||
class BaseInt(_BaseInt[int]): | |||
class BaseInt(_TraitType[SupportsIndex, int]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need _BaseInt
and _BaseFloat
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're still used in the CInt
and CFloat
trait types, I think. I looked at dealing with those ones too, but decided to limit the PR to avoid scope creep.
This PR improves the type stubs for Int and Float, being more accurate about what's accepted for those trait types. We need SupportsIndex, which is only available in typing from Python 3.8 onwards. So I've added typing-extensions as a dependency for traits-stubs. We can drop the dependency once we no longer support Python 3.6 and Python 3.7. (cherry picked from commit 26e6aaa)
This PR improves the type stubs for
Int
andFloat
, being more accurate about what's accepted for those trait types.We need
SupportsIndex
, which is only available intyping
from Python 3.8 onwards. So I've addedtyping-extensions
as a dependency fortraits-stubs
. We can drop the dependency once we no longer support Python 3.6 and Python 3.7.