Skip to content

BUG: fix bugs in io.votable related to numpy 2's representation of scalars - #16442

Merged
mhvk merged 1 commit into
astropy:mainfrom
neutrinoceros:io.votable/compat/16423/numpy2_scalar_repr
May 14, 2024
Merged

BUG: fix bugs in io.votable related to numpy 2's representation of scalars#16442
mhvk merged 1 commit into
astropy:mainfrom
neutrinoceros:io.votable/compat/16423/numpy2_scalar_repr

Conversation

@neutrinoceros

@neutrinoceros neutrinoceros commented May 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This pull request is to address part of #16423 (specifically, io.votable).
The first commit is common with #16426, #16427 and #16433 (allowing them to be merged in arbitrary order).
The fix itself is in part taken from #15065

  • 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.

@neutrinoceros neutrinoceros mentioned this pull request May 13, 2024
Merged
5 tasks done
@github-actions

github-actions Bot commented May 13, 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.

@github-actions

github-actions Bot commented May 13, 2024

Copy link
Copy Markdown
Contributor

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@neutrinoceros
neutrinoceros force-pushed the io.votable/compat/16423/numpy2_scalar_repr branch from 87fb0e8 to e624fdf Compare May 13, 2024 07:08
This was referenced May 13, 2024
Closed
Merged
@neutrinoceros
neutrinoceros marked this pull request as ready for review May 13, 2024 09:39
@neutrinoceros neutrinoceros changed the title BUG: Fix bugs in io.votable related to numpy 2's representation of scalars BUG: fix bugs in io.votable related to numpy 2's representation of scalars May 13, 2024
@pllim
pllim requested a review from tomdonaldson May 13, 2024 14:25
@pllim pllim added this to the v6.1.1 milestone May 13, 2024
@pllim

pllim commented May 13, 2024

Copy link
Copy Markdown
Member

pyvo and/or astroquery might want to review this. I will ping them.

mhvk
mhvk approved these changes May 13, 2024
View reviewed changes

@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.

Yes, that looks like a better fix than I had!

One question, but nothing depends on it, so I'll approve now.

Comment thread astropy/io/votable/tests/test_vo.py


@pytest.mark.xfail("legacy_float_repr")
@pytest.mark.usefixtures("without_legacy_printoptions")

@mhvk mhvk May 13, 2024

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.

Could we just do it for the whole test file?

@neutrinoceros neutrinoceros May 13, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In principle yes but there doesn't seem to be a need for it: tests that fail without legacy print options are actually pretty rare in the code base so I don't expect to find more when we remove the global setting, which we should be able to do as soon as we merge #15065

@neutrinoceros
neutrinoceros force-pushed the io.votable/compat/16423/numpy2_scalar_repr branch from e624fdf to 2bec9ce Compare May 14, 2024 06:25
@neutrinoceros

neutrinoceros commented May 14, 2024

Copy link
Copy Markdown
Contributor Author

The io.misc label should be removed.

@pllim pllim removed the io.misc label May 14, 2024
@mhvk

mhvk commented May 14, 2024

Copy link
Copy Markdown
Contributor

OK, let's get this in as well!

@mhvk
mhvk merged commit 918a531 into astropy:main May 14, 2024
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request May 14, 2024
@mhvk @meeseeksmachine
Backport PR astropy#16442: BUG: fix bugs in io.votable related to num…
97a6951 
…py 2's representation of scalars
@meeseeksmachine meeseeksmachine mentioned this pull request May 14, 2024
Merged
@neutrinoceros
neutrinoceros deleted the io.votable/compat/16423/numpy2_scalar_repr branch May 14, 2024 14:40
pllim added a commit that referenced this pull request May 14, 2024
@pllim
Merge pull request #16454 from meeseeksmachine/auto-backport-of-pr-16…
a5dc802 
…442-on-v6.1.x

Backport PR #16442 on branch v6.1.x (BUG: fix bugs in io.votable related to numpy 2's representation of scalars)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk approved these changes
@WilliamJamieson WilliamJamieson Awaiting requested review from WilliamJamieson
@matteobachetti matteobachetti Awaiting requested review from matteobachetti
@tomdonaldson tomdonaldson Awaiting requested review from tomdonaldson

Assignees

No one assigned

Labels

io.votable numpy-dev testing

Projects

None yet

Milestone

v6.1.1

Development

Successfully merging this pull request may close these issues.

3 participants

@neutrinoceros @pllim @mhvk