Remove Numpy restriction #392
Conversation
|
Warning Rate limit exceeded@jan-janssen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughBumped the numpy dependency from 1.26.4 to 2.3.3 in CI and project manifests: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.ci_support/environment.yml (1)
12-12: Unpinned NumPy in CI — pin to 1.26.4 or add 1.x/2.x CI matrixpyproject.toml pins numpy==1.26.4 but .ci_support/environment.yml lists "- numpy" (unpinned). Repo scan found no obvious NumPy‑2.0 incompatibilities (no np.int/float/bool/object/str, numpy.asscalar, numpy.matrix, .tostring(), or numpy.distutils). Either pin NumPy in .ci_support/environment.yml to 1.26.4 to match pyproject.toml, or add CI jobs testing NUMPY="1." and NUMPY="2." to detect regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.ci_support/environment.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unittest_matrix (ubuntu-latest, 3.10, linux-64-py-3-10)
- GitHub Check: unittest_matrix (macos-latest, 3.12, osx-64-py-3-12)
- GitHub Check: unittest_old
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #392 +/- ##
=======================================
Coverage 83.13% 83.13%
=======================================
Files 25 25
Lines 1862 1862
=======================================
Hits 1548 1548
Misses 314 314 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.ci_support/environment.yml (1)
20-20: sparse 0.17.0 supports NumPy 2.x — OK to keep in CI; consider making it optionalRelease notes confirm pydata/sparse 0.17.0 is NumPy 2.x compatible; if sparse is optional for the project, move it out of the base CI env or mark related tests to skip (see .ci_support/environment.yml entry
- sparse =0.17.0).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.ci_support/environment.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: unittest_matrix (ubuntu-latest, 3.11, linux-64-py-3-11)
- GitHub Check: unittest_matrix (macos-latest, 3.12, osx-64-py-3-12)
- GitHub Check: unittest_matrix (ubuntu-latest, 3.12, linux-64-py-3-12)
- GitHub Check: unittest_matrix (ubuntu-latest, 3.10, linux-64-py-3-10)
- GitHub Check: unittest_matrix (windows-latest, 3.12, win-64-py-3-12)
- GitHub Check: unittest_old
🔇 Additional comments (1)
.ci_support/environment.yml (1)
24-24: AI summary inconsistency: hatch‑vcs change not marked as added here.The AI summary says
hatch-vcs =0.4.0was added, but this line isn’t annotated as changed. Confirm whether it pre‑existed or was added in this PR.If it was added earlier, no action. If added now, consider whether it’s required in the CI runtime env vs. only for packaging steps.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
14-24: Python 3.9 support is now inconsistent with NumPy 2.x; update metadata.NumPy 2.x drops Python 3.9. With
numpy==2.2.6, advertisingrequires-python = ">=3.9, <3.13"and the 3.9 classifier is misleading and will break installs on 3.9.Apply this diff to align supported Python versions:
-requires-python = ">=3.9, <3.13" +requires-python = ">=3.10, <3.13" @@ - "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12",Optionally, if you intend to support Python 3.13 soon, consider dropping the
<3.13upper bound in a follow-up PR once CI covers it.Also applies to: 28-28
🧹 Nitpick comments (3)
pyproject.toml (2)
1-3: Avoid heavy runtime deps in build-system requires.
[build-system].requiresshould list only build tooling. Pullingase,numpy,scipyat build time slows builds and can fail in isolated environments (e.g., Py 3.9 + NumPy 2). If not strictly needed for building, move them to[project].dependenciesonly.Proposed change:
[build-system] -requires = ["ase", "numpy", "scipy", "hatchling==1.27.0", "hatch-vcs==0.5.0"] +requires = ["hatchling==1.27.0", "hatch-vcs==0.5.0"] build-backend = "hatchling.build"If any build step actually imports these libs, consider optional imports or lightweight stubs during build.
26-30: Pinning policy check (non-blocking).Exact pins for runtime deps can cause resolver conflicts for downstream users. Consider using compatible ranges (e.g., numpy>=2.2,<2.3) if project policy allows. Otherwise, keep as-is.
.ci_support/environment_mini.yml (1)
8-9: Hatch-VCS version mismatch between pyproject and CI env.pyproject.toml uses hatch-vcs==0.5.0; this env uses 0.4.0. Consider aligning to avoid build/versioning inconsistencies across environments.
Proposed change:
-- hatch-vcs =0.4.0 +- hatch-vcs =0.5.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.ci_support/environment.yml(1 hunks).ci_support/environment_mini.yml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .ci_support/environment.yml
🔇 Additional comments (1)
.ci_support/environment_mini.yml (1)
6-6: CI no longer references Python 3.9 or NumPy 1.xScanned .github and .ci_support — no '3.9' matches and no 'numpy =1.' / 'numpy==1.' pins found.
| dependencies = [ | ||
| "ase==3.26.0", | ||
| "numpy==1.26.4", | ||
| "numpy==2.2.6", | ||
| "scipy==1.15.2", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm downstream compatibility with NumPy 2.x removals.
NumPy 2 removed aliases/APIs (np.bool, np.int, np.float, np.object, np.asscalar, numpy.matrix, etc.). Please verify the codebase doesn’t rely on them.
Run this script to scan for common removed/deprecated NumPy symbols:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search typical removals in NumPy 2.x
rg -n -S -g '!**/tests/**' -C2 -t py '\bnp\.(bool|int|float|object|string_|unicode_|compat|asscalar)\b'
rg -n -S -g '!**/tests/**' -C2 -t py '\bnumpy\.(matrix|typeDict|set_printoptions\(.*legacy=.*?)\b'
rg -n -S -g '!**/tests/**' -C2 -t py '\b(np|numpy)\.load\([^)]*allow_pickle\s*=\s*True'
rg -n -S -g '!**/tests/**' -C2 -t py '\b(np|numpy)\.(fromstring|ndfromtxt)\('
rg -n -S -g '!**/tests/**' -C2 -t py '\b(np|numpy)\.(where|argwhere)\([^)]*==\s*None|\bis\s*None'
# Check version guards that may assume NumPy 1.x
rg -n -S -g '!**/tests/**' -C2 -t py 'numpy\.__version__'Length of output: 148
I scanned for typical removed aliases/APIs (np.bool/int/float/object/asscalar, numpy.matrix/typeDict, np.load(...allow_pickle=True), fromstring/genfromtxt, and numpy.__version__ checks) and found no matches. [resolve_review_comment]
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "Searching for removed/deprecated NumPy aliases (np.bool, np.int, np.float, np.object, np.asscalar, etc.)..."
rg -n -S -g '!**/tests/**' -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/venv/**' -g '!**/.venv/**' -C2 '\bnp\.(bool|int|float|object|str|unicode|long|complex|asscalar)\b' || true
echo "Searching for numpy.matrix / typeDict / set_printoptions(...legacy=...)..."
rg -n -S -g '!**/tests/**' -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/venv/**' -g '!**/.venv/**' -C2 '\bnumpy\.(matrix|typeDict|set_printoptions\(.*legacy=.*?)\b' || true
echo "Searching for np.load(..., allow_pickle=True)..."
rg -n -S -g '!**/tests/**' -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/venv/**' -g '!**/.venv/**' -C2 '\b(np|numpy)\.load\([^)]*allow_pickle\s*=\s*True' || true
echo "Searching for np.fromstring / np.genfromtxt usage..."
rg -n -S -g '!**/tests/**' -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/venv/**' -g '!**/.venv/**' -C2 '\b(np|numpy)\.(fromstring|genfromtxt)\(' || true
echo "Searching for numpy.__version__ checks..."
rg -n -S -g '!**/tests/**' -g '!**/.git/**' -g '!**/node_modules/**' -g '!**/venv/**' -g '!**/.venv/**' -C2 '\bnumpy\.__version__' || true
echo "Showing pyproject.toml (if present) lines around numpy dependency..."
rg -n -S -C2 'numpy' pyproject.toml || true
Length of output: 1527
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo top-level:"
ls -la
echo
echo "Listing up to 200 .py files:"
find . -type f -name '*.py' -not -path './.git/*' -not -path './venv/*' -not -path './.venv/*' -not -path './node_modules/*' | sed -n '1,200p' || true
echo
echo "Searching for numpy import statements..."
rg -n --hidden --no-ignore -S -C2 --glob '!**/.git/**' --glob '!**/node_modules/**' --glob '!**/venv/**' --glob '!**/.venv/**' 'import numpy as np|from numpy import|import numpy\b' || true
echo
echo "Searching for np.<removed aliases> uses..."
rg -n --hidden --no-ignore -S -C2 --glob '!**/.git/**' --glob '!**/node_modules/**' --glob '!**/venv/**' --glob '!**/.venv/**' '\bnp\.(bool|int|float|object|str|unicode|long|complex|asscalar|matrix)\b' || true
echo
echo "Searching for numpy.* removed/dangerous patterns..."
rg -n --hidden --no-ignore -S -C2 --glob '!**/.git/**' --glob '!**/node_modules/**' --glob '!**/venv/**' --glob '!**/.venv/**' '\bnumpy\.(matrix|typeDict|set_printoptions\(.*legacy|load\([^)]*allow_pickle\s*=\s*True|fromstring|genfromtxt|__version__)' || true
echo
echo "Searching for any 'np.' usages (for manual inspection)..."
rg -n --hidden --no-ignore -S -C2 --glob '!**/.git/**' --glob '!**/node_modules/**' --glob '!**/venv/**' --glob '!**/.venv/**' '\bnp\.' || true
echo
echo "pyproject.toml lines with numpy (sanity):"
sed -n '1,220p' pyproject.toml || trueLength of output: 6243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Scanning .py files (excluding tests, .git, venvs, node_modules) for NumPy-2 removed/deprecated symbols..."
python3 - <<'PY'
import os, re, sys
root = "."
exclude_subpaths = ("./tests/", "./.git/", "./venv/", "./.venv/", "./node_modules/", "./.tox/", "./.eggs/")
patterns = {
"np_aliases (np.bool/np.int/np.float/np.object/np.asscalar/etc.)": re.compile(r'\bnp\.(?:bool|int|float|object|str|unicode|long|complex|asscalar|matrix)\b'),
"numpy.matrix/typeDict": re.compile(r'\bnumpy\.(?:matrix|typeDict)\b'),
"set_printoptions(..., legacy=...)": re.compile(r'set_printoptions\([^)]*legacy\s*=', re.IGNORECASE),
"load(..., allow_pickle=True)": re.compile(r'\b(?:np|numpy)\.load\([^)]*allow_pickle\s*=\s*True', re.IGNORECASE),
"fromstring/genfromtxt": re.compile(r'\b(?:np|numpy)\.(?:fromstring|genfromtxt)\('),
"numpy.__version__": re.compile(r'\bnumpy\.__version__'),
}
matches = {k: [] for k in patterns}
def is_excluded(path):
for e in exclude_subpaths:
if path.startswith(e) or ('/'+e.strip('./')+'/') in path:
return True
return False
for dirpath, dirnames, filenames in os.walk(root):
# normalize dirpath
if is_excluded(dirpath + '/'):
continue
for fn in filenames:
if not fn.endswith('.py'):
continue
full = os.path.join(dirpath, fn)
# skip files in excluded subpaths
if is_excluded(full):
continue
try:
with open(full, 'r', encoding='utf-8', errors='replace') as f:
lines = f.readlines()
except Exception as e:
print(f"WARNING: cannot read {full}: {e}", file=sys.stderr)
continue
for i, line in enumerate(lines, start=1):
for name, pat in patterns.items():
if pat.search(line):
context_prev = lines[i-2].rstrip('\n') if i-2 >= 0 else ""
context = line.rstrip('\n')
context_next = lines[i].rstrip('\n') if i < len(lines) else ""
matches[name].append((full, i, context_prev, context, context_next))
any_match = False
for name, hits in matches.items():
if hits:
any_match = True
print(f"---- {name}: {len(hits)} match(es) ----")
for full, lineno, prev, line, nxt in hits:
print(f"{full}:{lineno}: {line}")
# print small context
if prev:
print(f" prev: {prev}")
if nxt:
print(f" next: {nxt}")
print()
if not any_match:
print("No matches found for checked patterns.")
else:
total = sum(len(h) for h in matches.values())
print(f"Total matches: {total}")
PYLength of output: 410
Replace numpy.matrix usage (removed in NumPy 2.x)
structuretoolkit/analyse/snap.py:265 — if np.matrix.trace(rotation_mat) != 3:; replace with if np.trace(rotation_mat) != 3: or if np.asarray(rotation_mat).trace() != 3: to restore NumPy 2.x compatibility.
🤖 Prompt for AI Agents
In structuretoolkit/analyse/snap.py around line 265, the code uses
np.matrix.trace(rotation_mat) which relies on the removed numpy.matrix API;
replace that call with either np.trace(rotation_mat) or
np.asarray(rotation_mat).trace() to ensure compatibility with NumPy 2.x, i.e.,
convert rotation_mat to a plain ndarray (if needed) and use np.trace(...) or its
ndarray.trace() method and keep the existing comparison to 3.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.ci_support/environment.yml (1)
12-12: Relax the NumPy pin to match SciPy’s supported range (or at least allow patch updates).
scipy==1.15.2supports NumPy>=1.23.5,<2.5.0, so=2.3.3is safe but unnecessarily strict and can cause avoidable solver churn. Consider either:
- Option A (preferred):
numpy >=2.0,<2.5(aligns with SciPy’s upper bound), or- Option B:
numpy =2.3.*(permit security/bugfix patches).Also ensure your CI exercises the min/max of the chosen range (e.g.,
2.0.xand latest2.3.x/2.4.x) to catch regressions early.References: SciPy toolchain table showing
1.15supports NumPy<2.5.0, and conda‑forge’s NumPy‑2 migration closure indicating broad 2.x availability; NumPy 2.3.3 release (2025‑09‑09). (docs.scipy.org)Suggested diffs:
- numpy =2.3.3 + numpy >=2.0,<2.5or
- numpy =2.3.3 + numpy =2.3.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.ci_support/environment.yml(1 hunks).ci_support/environment_mini.yml(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pyproject.toml
- .ci_support/environment_mini.yml
1 participant
Summary by CodeRabbit