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.

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

First fixes for NumPy 2.0 #1867

Merged
merged 3 commits into from Aug 31, 2023
Merged

Conversation

DougBurke
Copy link
Contributor

@DougBurke DougBurke commented Aug 30, 2023

Summary

Internal code changes to improve support for NumPy 2.0.

Details

Spurned on by

let's see what changes we can make now to future-proof ourselves.

With the dvelopment build of NumPy 2.0 installed

% pip install -U --pre --only-binary :all: -i https://pypi.anaconda.org/scientific-python-nightly-wheels/simple numpy

and removing matlpotlib and astropy (as the released versions of these I had, which are admittedly old, also have problems with NumPy 2.0) then most tests pass with this change. We do need to work out what to do with the change in string representation - e.g. #1864 (comment) - but this forms a nice limited-scope set of changes.

Let's see how it copes with some of the older NumPy versions we have in our CI runs! It's hard to check against numpy 2.0 since we don't have a CI run that provides this.

There will likely be more changes needed for our compiled extensions, but that can come later (and I don't know how to make pyproject.toml use a development build of NumPy rather than oldest-supported-numpy!).

Changes

  • use np.exceptions to import VisibleDeprecationWarning, falling back to the old location if not available
    • this is only needed for the tests
    • np.exceptions was added in NumPy 1.25
  • use np.float64 rather than np.float_
    • we could use np.double instead, but I think I like the choice of an explicit fixed-size datatype (since we have to deal with compiled code)
    • I didn't find documentation on when float_ and float64 become synonyms but I assume it holds for all recent NumPy versions
  • use np.bytes_ rather than np.string_
    • I think most uses of this are to handle the old behavior of crates/pyfits which would return byte strings at times, but I can not guarantee this, so I've just left a note for looking this at some future time
    • I didn't find documentation on when string_ and bytes_ become synonyms but I assume it holds for all recent NumPy versions
  • some minor code cleanups to use isinstance rather than an explicit type check

NumPy 1.25 introduced numpy.exceptions so use this for exceptions
when possible (needed for NumPy 2.0).
This is needed for NumPy 2.0 but is hopefully backwards-compatible
for the NumPy versions we care about.

We could have used numpy.double as the replacement, but I think it's
good to be explicit about the size of the type (e.g. 64-bit).
In most cases we just use np.bytes_ (needed for NumPy 2.0) along
with a note to check whether we still need this (historically we
have had issues from crates returning bytes rather than strings,
but it's not clear if this is still present).

One case I just removed it as it was only for a warning message and
it's not clear why it was being used.

There are several minor code cleanups related to this: use of
isinstance() rather than "type(..) in ...".

A few minor drive-by code clean ups to change

  if blah:
    do_something
  else:
    raise an_error

to

  if not blah:
    raise an_error

  do_something
@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #1867 (24ae928) into main (9eef8f9) will increase coverage by 0.00%.
The diff coverage is 81.08%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1867   +/-   ##
=======================================
  Coverage   81.23%   81.24%           
=======================================
  Files          73       73           
  Lines       26460    26463    +3     
  Branches     3962     3962           
=======================================
+ Hits        21496    21499    +3     
  Misses       4803     4803           
  Partials      161      161           
Files Changed Coverage Δ
sherpa/io.py 92.80% <0.00%> (ø)
sherpa/optmethods/ncoresde.py 40.98% <0.00%> (ø)
sherpa/optmethods/opt.py 70.80% <0.00%> (ø)
sherpa/utils/__init__.py 70.56% <33.33%> (ø)
sherpa/optmethods/optfcts.py 71.85% <87.50%> (ø)
sherpa/astro/ui/utils.py 97.95% <100.00%> (ø)
sherpa/conftest.py 93.05% <100.00%> (+0.08%) ⬆️
sherpa/instrument.py 87.23% <100.00%> (ø)
sherpa/optmethods/ncoresnm.py 31.92% <100.00%> (ø)
sherpa/ui/utils.py 96.09% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9eef8f9...24ae928. Read the comment docs.

@DougBurke DougBurke requested a review from hamogu August 31, 2023 13:17
@wmclaugh wmclaugh merged commit 319b53b into sherpa:main Aug 31, 2023
18 checks passed
@DougBurke DougBurke deleted the numpy-2-first-fixes branch August 31, 2023 17:14
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.

None yet

3 participants