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.

Sign up for GitHub

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

Jump to bottom

Revise guidance to require a minimum upper bound for supported ranks #702

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Revise guidance to require a minimum upper bound for supported ranks #702

merged 1 commit into from
Nov 30, 2023

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Nov 6, 2023

This PR

Related

@kgryte kgryte added the Narrative Content Narrative documentation content. label Nov 6, 2023
@kgryte kgryte added this to the v2023 milestone Nov 6, 2023
@kgryte
Copy link
Contributor Author

kgryte commented Nov 6, 2023

cc @leofang @Zac-HD

@kgryte kgryte mentioned this pull request Nov 6, 2023
Closed
17 tasks
@Zac-HD
Copy link

Zac-HD commented Nov 6, 2023

If array libraries have a consistent way to retrieve the maximum dimensionality they support, Hypothesis can provide more informative error messages. iirc Numpy has a MAX_DIMS constant somewhere...

@kgryte
Copy link
Contributor Author

kgryte commented Nov 6, 2023

@Zac-HD Indeed, that is part of the rationale behind #694. Once we merge the inspection API in, we'll add support for querying the maximum number of allowed dimensions.

rgommers
rgommers approved these changes Nov 16, 2023
View reviewed changes
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 LGTM, thanks @kgryte. I believe this aligns with previous discussions, and won't be a problem for any implementation (while 4 was the default maxdims supported by an implementation based on Kokkos IIRC).

@rgommers
Copy link
Member

This is pretty straightforward. I'll aim to merge this in a few days unless an objection appears.

@kgryte
Copy link
Contributor Author

kgryte commented Nov 30, 2023

As this PR has received multiple approvals and no objections, will go ahead and merge. Any further changes can be addressed in follow-up PRs...

@kgryte kgryte merged commit 5555d41 into data-apis:main Nov 30, 2023
3 checks passed
@kgryte kgryte deleted the fix/min-ndims branch November 30, 2023 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgommers rgommers rgommers approved these changes

@leofang leofang leofang approved these changes

Assignees
No one assigned
Labels
Narrative Content Narrative documentation content.
Projects
None yet
Milestone
v2023
Development

Successfully merging this pull request may close these issues.

Array objects of arbitrary rank are infeasible - require a reasonable range of ranks instead
4 participants
@kgryte @Zac-HD @rgommers @leofang