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

Use duck-typing to check for non-numeric types in approx() #8136

Closed

Conversation

nicoddemus
Copy link
Member

Fix #8132

def __abs__(self) -> float:
return abs(self.value)

def __sub__(self, other):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try type-annotating the return value like this:

Suggested change
def __sub__(self, other):
def __sub__(self, other: Any) -> Union[float, "Container"]:

However mypy complained with:

testing/python/approx.py:601:59: F821 undefined name 'Container'
testing/python/approx.py:608:60: F821 undefined name 'Container'

Is that because it is an inner class? Suggestions welcome. 👍

@jvansanten
Copy link
Contributor

I think the Python number hierarchy is still useful in ApproxScalar, simply because anything that inherits from that hierarchy is guaranteed to be a scalar whose comparison methods return bool. ndarray and its many work-alikes (such as tf.DeviceArray) are not scalars, and their comparison methods return something that is only convertible to bool when it has a single element. In other cases, the bool conversion raises ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all().. I put in an alternate proposal in #8137 that unifies the treatment of ndarray and its work-alikes, which I think would be less surprising to users. Feedback welcome!

@nicoddemus
Copy link
Member Author

Closing in favor of #8137.

@nicoddemus nicoddemus closed this Dec 13, 2020
@nicoddemus nicoddemus deleted the approx-duck-typing-8132 branch December 13, 2020 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pytest v6.2.0 pytest.approx failing tests when pytest v6.1.2 passes
2 participants