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

Align casting methods with Python behaviour #497

Merged
merged 15 commits into from
Dec 14, 2022

Conversation

honno
Copy link
Member

@honno honno commented Oct 18, 2022

Resolves #486 (adding x.__complex__()).

Per interest in having the builtin casting methods (__bool__/__int__/__float__/__complex__/__index__) support arrays of a different dtype kind, this PR also changes the language to clarify that adopters can support such casting. EDIT: now aligning casting methods with Python behaviour generally

(I opted against requiring cross-kind casting in this PR—see #486 (comment)) See #486 (comment)

@honno honno changed the title Add x.__complex__(), unrestrict cross-kind casting in respective array methods Align casting methods with Python behaviour Nov 2, 2022
@honno
Copy link
Member Author

honno commented Nov 2, 2022

Per what we decided upon in our last array meeting (see #486 (comment)), I've updated this PR with my take on how to we align the spec's casting methods with Python's behaviour.

I have some thorough testing for how Python's casting works in https://gist.github.com/honno/98371048b4d5b9fc4a003377a714e827, which I'll have to tidy up (will be useful anyway for the test suite).

As @seberg noted, NumPy seems to 100% follow Python casting behaviour for its arrays (if we're not including a small divergence in behaviour for NumPy scalars). Only notable thing maybe is NumPy not aligning with Python's __index__ (not actually casting I suppose), in that it raises TypeError for bool arrays, where Python bool objects just returns itself.

Not 100% on language used, happy to hear what folk think. Will sleep on it anyway.

Copy link
Member Author

@honno honno left a comment

Choose a reason for hiding this comment

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

Just realised I should probably special case non-finite values for __bool__.

spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
@honno
Copy link
Member Author

honno commented Nov 14, 2022

Think this is ready @seberg @asmeurer? With the follow up of specifying <bool-array>.__index__() behaviour as erroneous (context #497 (comment)).

spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
spec/API_specification/array_api/array_object.py Outdated Show resolved Hide resolved
@honno
Copy link
Member Author

honno commented Nov 15, 2022

Note previews are broken because of some work I was doing in my fork for #505, but you can change the URLs to lead with .../0/build/draft/index.html like so still.

@honno honno requested a review from kgryte November 17, 2022 11:05
@kgryte kgryte added this to the v2022 milestone Nov 21, 2022
@kgryte kgryte added the API change Changes to existing functions or objects in the API. label Dec 14, 2022
@kgryte
Copy link
Contributor

kgryte commented Dec 14, 2022

I believe all discussion points have been resolved. I'll go ahead and merge. Any further changes can be addressed in follow-up PRs. Thanks, all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Changes to existing functions or objects in the API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add __complex__ to array object
5 participants