Adjust to numpy dev scalar format #15065

mhvk · 2023-07-12T19:53:46Z

DEV NOTE: Do not merge until numpy 2.0 is actually out! ~~Do not merge until this is merged and available in numpy nightly wheel (when we start to see things fail in devdeps)~~

Revert TST: Use legacy numpy 1.25 print #15096 if it got in first. (EDIT: it should get in first; this one is for when 2.0 is actually out)
Decide for scalar quantity.value whether it should not just return an array scalar (see below)
Check whether we may want similar adjustments elsewhere (maybe as follow-up; e.g., Structure for structure units output) - but this should not be part of this PR, so punted to Post-numpy-2.0 changes in repr/str #15793.
Decide whether we are happy for the docs to pass tests only on numpy >=2.0. If not, we have to update doctest-plus to ignore np.float64, etc., in comparing numerical values. 2023-Dec: seems that we are heading to a docs-on-numpy>=2.0 only state

This is a start to adjusting our tests for the change in how scalars are formatted in NEP 51 - see numpy/numpy#22449

~~Note that it does not adjust tests of repr and the documentation yet!~~

Posting this in part to show that outside of repr this is not a big deal.

In the documentation, one often will have things like

>>> q = 5 * u.m
>>> q.value
5.0  -> np.float64(5.0)

This brings up a more general question of whether we should use this opportunity of the repr changing to just return an array scalar (i.e., it would be np.array(5.0) instead).

Fix #15095
Fix #15792

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 · 2023-07-12T19:54:29Z

github-actions · 2023-07-12T19:54:35Z

👋 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?

pllim · 2023-07-12T21:33:36Z

The diff looks much simpler than I anticipated. Thanks! 🤞

mhvk · 2023-07-12T23:59:05Z

Well, the main diff from the docs is not yet in there...

pllim · 2023-07-26T17:56:04Z

If #15096 goes in first, that PR also has to be reverted here. FYI.

mhvk · 2023-07-26T19:20:27Z

I just rebased and force-pushed to get a fresh test run. If -dev passes, we probably should run extra CI as well, just in case.

p.s. Very pressed for time right now, so if this doesn't converge easily, happy with #15096 as a stop-gap measure...

pllim · 2023-07-26T19:35:51Z

Alas... still 95 failed (though 3 of them are from modeling and unrelated).

pllim · 2023-07-26T19:36:34Z

Looks like we need version check in some places?

ImportError: cannot import name 'get_formatter' from 'numpy.core.arrayprint'

mhvk · 2023-07-26T20:45:51Z

Ah, right, that changed, we didn't make the new get_formatter in the end! Can be a quick change (hopefully)...

pllim · 2023-07-26T21:15:01Z

There are 51 failures now (3 unrelated, the rest doctest output comparison failures). I actually don't like the new look in our own doc test output... especially when you print out the WCS... it looks jarring and adds noise to the actual data I am interested in (say, what is the value of CVAL1, without also have to see it is np.float because I already know that).

mhvk · 2023-08-11T16:20:24Z

The problem with this PR is that all doctests will fail on older numpy. In principle, we can just not run the tests on older versions, but that makes sense only once 2.0 is actually out. I think the logical route is to go with #15096 first, so that things work again, and then do this one once 2.0 is released.

pllim · 2023-08-11T16:30:22Z

In that case might even want to wait for numpy 2.1 for a chance for users to catch up? Or is that too long?

pllim · 2023-08-11T16:32:33Z

astropy/units/structured.py

@@ -121,7 +121,7 @@ class StructuredUnit:
    Structured units share most methods with regular units::

        >>> su.physical_type
-        ((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), PhysicalType('time'))
+        astropy.units.structured.Structure((astropy.units.structured.Structure((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), dtype=[('f0', 'O'), ('f1', 'O')]), PhysicalType('time')), dtype=[('f0', 'O'), ('f1', 'O')])


How does @namurphy and @nstarman feel about this?

Yes, that was one case where I felt we might want to override __repr__... I'll put it on top.

Agreed! I think the current ((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), PhysicalType('time')) should be the behaviour of __str__.
For __repr__ we at least don't want to show the module structured.

Suggested change

astropy.units.structured.Structure((astropy.units.structured.Structure((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), dtype=[('f0', 'O'), ('f1', 'O')]), PhysicalType('time')), dtype=[('f0', 'O'), ('f1', 'O')])

astropy.units.Structure((astropy.units.Structure((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), dtype=[('f0', 'O'), ('f1', 'O')]), PhysicalType('time')), dtype=[('f0', 'O'), ('f1', 'O')])

Agreed, that is the minimal change. But coming back to this fairly substantial PR, it is probably best to do that afterwards - I raised a new issue at #15793.

Totally fine by me. I can review this PR in a few days, if you're ready @mhvk

Thanks! This PR is basically waiting for numpy 2.0 to be released... I'm just keeping it up to date once in a while until that happens.

mhvk · 2024-05-11T21:22:26Z

While testing this with test-devdeps-alldeps, I found some tests with pandas failed.

See fix in #16439. Combined with @larrybradley's fixes in #16431 that I already rebased on, everything should pass, which will mean this can go out of draft. Note that #16439 will bring in a devdeps-alldeps run in extra CI that would be good for a final sanity check.

neutrinoceros · 2024-05-13T07:02:09Z

astropy/io/votable/tests/test_vo.py

@@ -266,6 +266,8 @@ def test_iter_coosys(self):
 )
 def test_select_missing_columns_error_message(columns, expected_missing):
    # see https://github.com/astropy/astropy/pull/15956
+    # Turn into numpy strings (otherwise match fails on numpy >= 2.0).
+    expected_missing = [np.str_(e) for e in expected_missing]


I'm don't think this is the correct fix here. I'll link to my own solution when I open the PR.

mhvk added the numpy-dev label Jul 12, 2023

mhvk added this to the v6.0 milestone Jul 12, 2023

github-actions bot added io.votable table io.fits units utils io.misc utils.masked labels Jul 12, 2023

mhvk mentioned this pull request Jul 12, 2023

ENH: Update scalar representations as per NEP 51 numpy/numpy#22449

Merged

4 tasks

This was referenced Jul 26, 2023

TST: numpy 2.0 float repr incompatibility in tests astropy/pyvo#464

Closed

TST: Use legacy numpy 1.25 print #15096

Merged

mhvk force-pushed the adjust-to-numpy-dev-scalar-format branch from 28e80f7 to cf64e6c Compare July 26, 2023 19:16

braingram mentioned this pull request Jul 27, 2023

convert numpy scalars to python types before yaml encoding asdf-format/asdf#1605

Merged

eerovaher mentioned this pull request Jul 31, 2023

Cosmo param dataclass #14874

Merged

pllim mentioned this pull request Aug 1, 2023

TST: Handle new numpy 2.0.dev string representation scientific-python/pytest-doctestplus#210

Open

1 task

mhvk force-pushed the adjust-to-numpy-dev-scalar-format branch from ffffe25 to a6c39c6 Compare August 11, 2023 14:31

pllim reviewed Aug 11, 2023

View reviewed changes

mhvk added 21 commits May 11, 2024 13:30

Table test touchups

a2ae3fe

Time touchups

d46efde

Timeseries touchups

29b91f0

Units touchup

5d4c20d

Small masked fix for structured scalars

d84166e

Cosmology docs updates

6b1549a

Coordinates doc updates

6b1ba0d

Modeling doc updates

44bc1c6

NDData docs update

07f6a7a

Stats doc updates

032aded

Table doc updates

6a24e88

Time doc updates

5a79ae5

Timeseries doc updates

6a11fae

Units doc updates

b81325d

Utils doc updates

fc8899d

Visualization doc updates

fbbf9fb

WCS doc updates

61f09cf

known issues doc updates

87a750d

Mark .rst and some py files with docstrings as numpy>=2.0 for doctests

65905d9

Make all of io.fits docs compatible with numpy 2.0

3e2b25e

Small io.votable test fixup

3db12fa

mhvk mentioned this pull request May 11, 2024

TST: adapt table representation tests to numpy 2 #16433

Open

1 task

mhvk force-pushed the adjust-to-numpy-dev-scalar-format branch from 2dfcd6f to 3db12fa Compare May 11, 2024 20:42

neutrinoceros mentioned this pull request May 12, 2024

BUG: fix compatibility with numpy 2 in diff reports (io.fits, utils) #16426

Merged

1 task

mhvk mentioned this pull request May 12, 2024

Fix pandas to Table conversion failures with numpy-dev #16439

Merged

1 task

neutrinoceros reviewed May 13, 2024

View reviewed changes

This was referenced May 13, 2024

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

Merged

BUG: fix bugs with how masked structured arrays were represented with numpy 2 #16443

Merged

neutrinoceros mentioned this pull request May 23, 2024

Reminder: clean up without_legacy_printoptions test decorator #16491

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust to numpy dev scalar format #15065

Adjust to numpy dev scalar format #15065

mhvk commented Jul 12, 2023 •

edited

github-actions bot commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

pllim commented Jul 12, 2023

mhvk commented Jul 12, 2023

pllim commented Jul 26, 2023

mhvk commented Jul 26, 2023

pllim commented Jul 26, 2023

pllim commented Jul 26, 2023

mhvk commented Jul 26, 2023

pllim commented Jul 26, 2023

mhvk commented Aug 11, 2023

pllim commented Aug 11, 2023

pllim Aug 11, 2023

mhvk Aug 11, 2023

nstarman Dec 27, 2023

mhvk Dec 27, 2023

nstarman Dec 28, 2023

mhvk Dec 28, 2023

mhvk commented May 11, 2024

neutrinoceros May 13, 2024

neutrinoceros May 13, 2024

	astropy.units.structured.Structure((astropy.units.structured.Structure((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), dtype=[('f0', 'O'), ('f1', 'O')]), PhysicalType('time')), dtype=[('f0', 'O'), ('f1', 'O')])
	astropy.units.Structure((astropy.units.Structure((PhysicalType('length'), PhysicalType({'speed', 'velocity'})), dtype=[('f0', 'O'), ('f1', 'O')]), PhysicalType('time')), dtype=[('f0', 'O'), ('f1', 'O')])

Adjust to numpy dev scalar format #15065

Are you sure you want to change the base?

Adjust to numpy dev scalar format #15065

Conversation

mhvk commented Jul 12, 2023 • edited

github-actions bot commented Jul 12, 2023

github-actions bot commented Jul 12, 2023

pllim commented Jul 12, 2023

mhvk commented Jul 12, 2023

pllim commented Jul 26, 2023

mhvk commented Jul 26, 2023

pllim commented Jul 26, 2023

pllim commented Jul 26, 2023

mhvk commented Jul 26, 2023

pllim commented Jul 26, 2023

mhvk commented Aug 11, 2023

pllim commented Aug 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhvk commented May 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhvk commented Jul 12, 2023 •

edited