Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the project to be compatible with NumPy 2.x by loosening the NumPy upper bound, refreshing lockfiles to a NumPy 2.4.3 resolution, replacing deprecated numpy.trapz usage, and attempting to prevent an insecure mdsthin fallback when MDSplus fails due to NumPy-related import problems.
Changes:
- Unpin NumPy (
numpy>=1.26.0) and update lockfiles to resolve to NumPy 2.4.3. - Replace
numpy.trapzwithscipy.integrate.trapezoidin code paths that integrate signals. - Add logic intended to detect NumPy-related
MDSplusimport failures and raise instead of falling back tomdsthin.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Removes NumPy <2.0.0 upper bound to allow NumPy 2+. |
poetry.lock |
Updates locked packages to a NumPy 2.4.3 environment (incl. netcdf4 metadata changes). |
uv.lock |
Updates uv lock resolution to NumPy 2.4.3 and removes NumPy <2.0.0 pin. |
disruption_py/settings/time_setting.py |
Replaces np.trapz with scipy.integrate.trapezoid. |
disruption_py/machine/cmod/thomson.py |
Replaces np.trapz and updates SciPy import usage accordingly. |
disruption_py/inout/mds.py |
Adds NumPy-related import failure detection to avoid mdsthin fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ZanderKeith
left a comment
There was a problem hiding this comment.
Confirmed results for the physics methods are unchanged between numpy 1.26.4, 2.0.2, and 2.4.4.
yumouwei
left a comment
There was a problem hiding this comment.
I also got the error with MDSplus on omega so I'll wait for your fix.
|
what did you get? maybe this new error while testing out the branch? disruption-py/disruption_py/inout/mds.py Line 28 in dadb1df I've just updated the "public installations", please retry and LMK! |
|
I reran all tests on all machines for several
where: 🟩 = fast tests, 🟢 = full tests. |
|
Yes I got the Can confirm the public installation works without running into the error. |
|
it's now based on the export PYTHONPATH="$DISPY_DIR/mdsplus/stable/python:$PYTHONPATH"please try and test as I've carried out all tests from my part and would probably merge later today. |
|
I tested using your MDSplus installation with my private dis-py installation and it worked. |
numpy.trapzwithscipy.integrate.trapezoid,Note
trapzwas deprecated in v2.0 and renamed totrapezoid, and eventually dropped in v2.4.Caution
this will definitely break DIII-D workflows due to their old MDSplus version (7.139.59).
I'll push a fix -- i.e. a newer MDSplus version -- upon merge for my "public installation"...