Skip to content

MAINT: Use array-api-strict instead of np.array_api - #16150

Merged
mhvk merged 1 commit into
astropy:mainfrom
MridulS:use_array-api-strict
Mar 5, 2024
Merged

mhvk merged 1 commit into
astropy:mainfrom
MridulS:use_array-api-strict

Conversation

@MridulS

@MridulS MridulS commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

Description

This fixes #16149
I think we should just add this optional dependency (it's a light python package) instead of skipping the test all together. If we are moving towards an array api astropy future we would need to add this dependency soon enough :)

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@github-actions

github-actions Bot commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@neutrinoceros

Copy link
Copy Markdown
Contributor

Thanks ! I think we can just close #16148 in favor of this one

@pllim

pllim commented Mar 4, 2024

Copy link
Copy Markdown
Member

Thanks! Let me run the "exotic archs" also, just in case.

@pllim

pllim commented Mar 4, 2024 •

Copy link
Copy Markdown
Member

devdeps failure is unrelated (but also kinda bad because we want to test this on devdeps, right?)

@pllim

pllim commented Mar 4, 2024

Copy link
Copy Markdown
Member

How important is it to wait for #16142 to get merged first and then rebase, so you can be sure this works with numpy-dev?

@neutrinoceros

Copy link
Copy Markdown
Contributor

If it helps, I just took out controversial parts from #16142 so it's more lightweight and hopefully backportable now :)

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks! I agree that we should keep the test, since this was added because of a specific issue. And adding the dependency to test_all seems just right.

@pllim

pllim commented Mar 4, 2024

Copy link
Copy Markdown
Member

Any new Array API changes are unlikely to be backported anyway, right? Is there a reason why this PR needs to be backported?

@mhvk

mhvk commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

@pllim - it does not need to be backported, since it only affects the test added in #15752, which was for 6.1.

@pllim

pllim commented Mar 4, 2024

Copy link
Copy Markdown
Member

OK. I'll let you decide when to merge. Thanks, all!

@mhvk
mhvk merged commit d3a39fa into astropy:main Mar 5, 2024
@mhvk

mhvk commented Mar 5, 2024

Copy link
Copy Markdown
Contributor

Thanks, @MridulS!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reminder: adapt numpy.array_api test to use array-api-strict instead

4 participants