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
MNT: more compatibility with NumPy 2.0 #15235
Changes from all commits
8fe23a7
0ef26d8
236d90d
61b6d15
db813c4
05e9be7
b6deb07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import io | ||
import os | ||
import re | ||
import sys | ||
from contextlib import nullcontext | ||
from io import BytesIO | ||
from textwrap import dedent | ||
|
@@ -1243,7 +1244,7 @@ def test_data_out_of_range(parallel, fast_reader, guess): | |
fast_reader["parallel"] = parallel | ||
if fast_reader.get("use_fast_converter"): | ||
rtol = 1.0e-15 | ||
elif np.iinfo(np.int_).dtype == np.dtype(np.int32): | ||
elif (sys.maxsize < 2**32) or (sys.platform == "win32"): | ||
# On 32bit the standard C parser (strtod) returns strings for these | ||
pytest.xfail("C parser cannot handle float64 on 32bit systems") | ||
|
||
|
@@ -1358,7 +1359,7 @@ def test_data_at_range_limit(parallel, fast_reader, guess): | |
fast_reader["parallel"] = parallel | ||
if fast_reader.get("use_fast_converter"): | ||
rtol = 1.0e-15 | ||
elif np.iinfo(np.int_).dtype == np.dtype(np.int32): | ||
elif (sys.maxsize < 2**32) or (sys.platform == "win32"): | ||
# On 32bit the standard C parser (strtod) returns strings for these | ||
pytest.xfail("C parser cannot handle float64 on 32bit systems") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another case where we don't even seem to need the
E Failed: DID NOT WARN. No warnings of type (<class 'Warning'>,) were emitted. while the test below in L1410 actually expects There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But there isn't any new pytest release this past weekend. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this would have broken already when changing this from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the Windows job really 32-bit though? The log says There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is passing all the reads up to e.g. https://github.com/astropy/astropy/actions/runs/5997690440/job/16264575442#step:10:1868 |
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -252,7 +252,6 @@ def _represent_tuple(self, data): | |
# Numpy dtypes | ||
AstropyDumper.add_representer(np.bool_, yaml.representer.SafeRepresenter.represent_bool) | ||
for np_type in [ | ||
np.int_, | ||
np.intc, | ||
np.intp, | ||
np.int8, | ||
|
@@ -267,11 +266,11 @@ def _represent_tuple(self, data): | |
AstropyDumper.add_representer( | ||
np_type, yaml.representer.SafeRepresenter.represent_int | ||
) | ||
for np_type in [np.float_, np.float16, np.float32, np.float64, np.longdouble]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So for users with numpy versions below 2 that used such data types, these will not be supported anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, thanks for the clarification! |
||
for np_type in [np.float16, np.float32, np.float64, np.longdouble]: | ||
AstropyDumper.add_representer( | ||
np_type, yaml.representer.SafeRepresenter.represent_float | ||
) | ||
for np_type in [np.complex_, complex, np.complex64, np.complex128]: | ||
for np_type in [complex, np.complex64, np.complex128]: | ||
AstropyDumper.add_representer(np_type, _complex_representer) | ||
|
||
AstropyLoader.add_constructor("tag:yaml.org,2002:python/complex", _complex_constructor) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,6 @@ | |
from astropy.modeling.core import Fittable1DModel | ||
from astropy.modeling.parameters import Parameter | ||
from astropy.utils import minversion | ||
from astropy.utils.compat.numpycompat import NUMPY_LT_2_0 | ||
from astropy.utils.compat.optional_deps import HAS_SCIPY | ||
from astropy.utils.exceptions import AstropyUserWarning | ||
|
||
|
@@ -184,6 +183,7 @@ def test_bounds_slsqp(self): | |
assert intercept + 10**-5 >= bounds["intercept"][0] | ||
assert intercept - 10**-5 <= bounds["intercept"][1] | ||
|
||
@pytest.mark.filterwarnings("ignore:The fit may be unsuccessful") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WilliamJamieson , if you are not okay with this, please open follow-up PR. Thanks! |
||
@pytest.mark.parametrize("fitter", fitters) | ||
def test_bounds_gauss2d_lsq(self, fitter): | ||
fitter = fitter() | ||
|
@@ -204,21 +204,12 @@ def test_bounds_gauss2d_lsq(self, fitter): | |
theta=0.5, | ||
bounds=bounds, | ||
) | ||
if isinstance(fitter, (fitting.LevMarLSQFitter, fitting.DogBoxLSQFitter)): | ||
with pytest.warns(AstropyUserWarning, match="The fit may be unsuccessful"): | ||
model = fitter(gauss, X, Y, self.data) | ||
if isinstance(fitter, fitting.TRFLSQFitter): | ||
ctx = np.errstate(invalid="ignore", divide="ignore") | ||
else: | ||
ctx2 = nullcontext() | ||
if isinstance(fitter, fitting.TRFLSQFitter): | ||
ctx = np.errstate(invalid="ignore", divide="ignore") | ||
if not NUMPY_LT_2_0 or not SCIPY_LT_1_11_2: | ||
ctx2 = pytest.warns( | ||
AstropyUserWarning, match="The fit may be unsuccessful" | ||
) | ||
else: | ||
ctx = nullcontext() | ||
with ctx, ctx2: | ||
model = fitter(gauss, X, Y, self.data) | ||
ctx = nullcontext() | ||
with ctx: | ||
model = fitter(gauss, X, Y, self.data) | ||
x_mean = model.x_mean.value | ||
y_mean = model.y_mean.value | ||
x_stddev = model.x_stddev.value | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
|
||
import numpy as np | ||
|
||
from astropy.utils.compat import NUMPY_LT_2_0 | ||
from astropy.utils.data_info import ParentDtypeInfo | ||
from astropy.utils.shapes import NDArrayShapeMethods | ||
|
||
|
@@ -759,9 +760,17 @@ | |
else: | ||
# Parse signature with private numpy function. Note it | ||
# cannot handle spaces in tuples, so remove those. | ||
in_sig, out_sig = np.lib.function_base._parse_gufunc_signature( | ||
ufunc.signature.replace(" ", "") | ||
) | ||
if NUMPY_LT_2_0: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mhvk , this might not be as clean as you like based on your previous changes. If this bothers you, please open follow-up PR. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries, that looks fine! Thanks! |
||
in_sig, out_sig = np.lib.function_base._parse_gufunc_signature( | ||
ufunc.signature.replace(" ", "") | ||
) | ||
else: | ||
( | ||
in_sig, | ||
out_sig, | ||
) = np.lib._function_base_impl._parse_gufunc_signature( | ||
ufunc.signature.replace(" ", "") | ||
) | ||
axis = kwargs.get("axis", -1) | ||
keepdims = kwargs.get("keepdims", False) | ||
in_masks = [] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could imagine that the windows failures are related to this change. I just got the
sys.maxsize
replacement from stackoverflow. But I guess on 32bit one could also testif np.intp == np.int32
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is obviously not triggering the
pytest.xfail
, but it is also not failing – just missing 1 expected warning, so maybe that part should instead be changed below.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest refuses to run locally on Windows for other reasons and I don't have time to fix it. I think to match existing behavior, it is best to just xfail if win32 is detected. I added a commit to do this.