Skip to content

PR: Add can-cast, broadcast-arrays and broadcast-to #132

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 11 commits into from
Apr 19, 2021

Conversation

steff456
Copy link
Member

@steff456 steff456 commented Feb 22, 2021

This PR

  • Adds specification for can_cast, which determines if a type conversion is allowed.
  • Adds specification for broadcast_arrays, which broadcast any number of arrays against each other.
  • Adds specification for broadcast_to, which broadcast an array to a new shape.

Notes

@steff456 steff456 requested review from rgommers and kgryte February 22, 2021 22:25
@steff456 steff456 self-assigned this Feb 22, 2021
@rgommers
Copy link
Member

Thanks @steff456. Could you add a Notes section to the PR description describing any differences between libraries? As well as links to signature comparisons (e.g. https://github.com/data-apis/array-api-comparison/blob/59a48db415a1eae754e7feaa8d340b1d6f2b2786/signatures/data-types/can_cast.md)?

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.

This needs some changes; the most important points need comparison data before we can decide.

@asmeurer
Copy link
Member

I noted some issues here that came up from my script that parses the function signatures. There are also similar issues in iinfo and finfo, if you want to fix them here (otherwise I will open a new PR for them). Some comments were marked as resolved but weren't actually resolved.

@steff456
Copy link
Member Author

I can update that in this PR. I haven't make a push with the changes but I already made them locally (:

@asmeurer
Copy link
Member

I have #133 but feel free to make the changes here too if you already made them.

@rgommers rgommers added the API extension Adds new functions or objects to the API. label Mar 20, 2021
Copy link
Contributor

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @steff456! Mainly formatting and phrasing nits. The main question for me is whether can_cast should support dtypes and arrays for both from and to.

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.

LGTM too, merging. Thanks @steff456. And thanks for the review @kgryte

@kgryte kgryte deleted the add-can-cast branch March 8, 2023 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants