Skip to content

Update typing for array object dunder methods #173

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

Merged
merged 1 commit into from
May 3, 2021
Merged

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Apr 26, 2021

This PR

  • updates type hints for various array object dunder methods, as mentioned in gh-143 (comment).
  • updates parameter descriptions to match element-wise counterparts (e.g., MUST vs SHOULD, etc).

@kgryte kgryte changed the title Updating typing for array object dunder methods Update typing for array object dunder methods Apr 26, 2021
@rgommers rgommers added the Maintenance Bug fix, typo fix, or general maintenance. label Apr 27, 2021
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @kgryte. LGTM.

Technically int is a subtype of float so not necessary to include (and there's a mypy guideline saying to not do Union[int, float] when you can simply do float, however for clarity I think it's preferred to do so here anyway.

@rgommers rgommers merged commit 2e759ef into main May 3, 2021
@rgommers rgommers deleted the dunder-typings branch May 3, 2021 20:18
@asmeurer
Copy link
Member

asmeurer commented May 4, 2021

Technically int is a subtype of float so not necessary to include (and there's a mypy guideline saying to not do Union[int, float] when you can simply do float, however for clarity I think it's preferred to do so here anyway.

__pow__ and __divide__ in the spec currently only require floating-point, meaning for the NumPy API, they error on int inputs. Is that actually correct behavior based on the spec? If so, this is a breakage from the mypy typing. Does mypy have a way to specify float but not int?

@rgommers
Copy link
Member

rgommers commented May 4, 2021

__pow__ and __divide__ in the spec currently only require floating-point, meaning for the NumPy API, they error on int inputs.

That does sound right, x**2 should work right? Only negative integers will error. Which is not something easy to express in type annotation (which is fine).

Does mypy have a way to specify float but not int?

Not that I know of.

@asmeurer
Copy link
Member

asmeurer commented May 4, 2021

Actually, I didn't notice that the spec says "numeric data type" for __pow__ and __truediv__ but "floating-point data type" for pow and divide. So I presently had them not working for integer inputs, but I should change this. Or the spec should be fixed (it doesn't mention anything about negative integers for __pow__).

@rgommers
Copy link
Member

rgommers commented May 5, 2021

Ah that should be made consistent. And negative integers should be disallowed I believe.

asmeurer added a commit to asmeurer/array-api that referenced this pull request Jul 9, 2021
Previously it was any numeric dtype, but this was inconsistent with the
functions pow() and divide(). Integer inputs are problematic because they
would either always have to be cast to float or use value-based casting
depending on, e.g., the sign of the exponent for __pow__. Furthermore, in
order to properly correspond to Python conventions, __truediv__ should always
return a float (integer division should be implemented by __floordiv__).

This was discussed in data-apis#173.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Bug fix, typo fix, or general maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants