-
Notifications
You must be signed in to change notification settings - Fork 30
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
Use numpy>=2 #613
Use numpy>=2 #613
Conversation
@krzywon Thanks for this PR, just wondering, will this PR be reviewed or merged? |
@bobleesj, a review is required before a merge can be made into sasmodels, but that review can come from anyone who wasn't the original PR creator. Would you feel comfortable, and have the time to review it? |
@krzywon yes happy to review. Will do later in the afternoon. |
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 am wondering reasons preventing sasmodels
from testing against Python 3.12 in the test.yml workflow?
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.
Great question! I don't think there is anything holding us back. I'm going to add 3.12 and see what happens.
@@ -29,7 +29,7 @@ def find_version(package): | |||
return version[1:-1] | |||
raise RuntimeError("Could not read version from %s/__init__.py"%package) | |||
|
|||
install_requires = ['numpy==1.*', 'scipy'] | |||
install_requires = ['numpy', 'scipy'] |
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.
great
@@ -36,8 +36,7 @@ jobs: | |||
python -m pip install --upgrade pip | |||
python -m pip install wheel setuptools | |||
python -m pip install mako | |||
python -m pip install numpy==1.* |
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.
great
@@ -70,8 +70,8 @@ def find_version(package): | |||
}, | |||
install_requires=install_requires, | |||
extras_require={ | |||
'full': ['docutils', 'bumps', 'matplotlib', 'columnize'], | |||
'server': ['bumps'], | |||
'full': ['docutils', 'bumps==0.*', 'matplotlib', 'columnize', 'siphash24'], |
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.
since I am new here, just wondering why bumps
and siphash24
were modified
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.
siphash24
was optionally required for a set of dependencies, but these changes move to a point where it is now required, which is why I've added it here. Without it, our tests were failing.
bumps
is our optimizing package (https://github.com/bumps/bumps). v1.0 of bumps has made significant, breaking, changes that will create issues with packages that depend on sasmodels. This is a temporary fix to maintain compatibility.
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.
Got it, thanks for the explanation!
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.
@krzywon reviewed. Could you please check my comments?
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.
Thanks for the review @bobleesj . This also looks good to me.
@krzywon Great! 3.12 is working here. For our group's package, we also plan to support Python 3.13. If it's not too much trouble, could you please check if 3.13 works as well? |
btw, I am not sure who has write access and can provide a review. Are either of the Pauls (@pkienzle @butlerpd ) or Matthieu (@mdoucet ) still actively maintaining this? conda-forge is already shipping python 3.13 as latest so we are migrating all our diffpy codes over. I just am wondering who is leading the maintenance on here these days? |
@bobleesj, I've added the py3.13 test, as requested. Let's see what happens! @sbillinge, I spoke to @pkienzle, and he says he will take a look when he has a chance. |
I rolled back from py3.13. The build is failing to find a compatible pyopencl wheel. I could probably build from source, but that is beyond the scope of this PR. |
Yes. Thanks a ton! |
For reference, https://numpy.org/doc/stable/numpy_2_0_migration_guide.html |
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.
Oops... thought I approved this already. Sorry!
This allows sasmodels to use numpy>=2. This also fixes two sphinx warnings that were happening during the build.
Fixes #604
Fixes #598
Refs #609