Declare directly-imported dependencies; cap numpy<2.5 - #1145
Declare directly-imported dependencies; cap numpy<2.5#1145AlexanderFengler wants to merge 1 commit into
Conversation
Closes #1144 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughChangesRuntime dependency constraints
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 38-39: Validate the dependency pairing represented by the jax and
jaxonnxruntime entries in pyproject.toml: test the onnx2jax.py import path with
the resolved versions, or constrain jax to a range verified compatible with
jaxonnxruntime>=0.3.0. Preserve the jaxonnxruntime dependency because it remains
actively imported.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| "jax>=0.7.0", | ||
| "jaxonnxruntime>=0.3.0", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
for path in pyproject.toml src/hssm tests; do
if [ -e "$path" ]; then
rg -n -C 4 'jaxonnxruntime|import jax|from jax' "$path" || true
fi
done
python - <<'PY'
import json
from urllib.request import urlopen
for package, version in (("jaxonnxruntime", "0.3.0"), ("jax", "0.7.0")):
with urlopen(f"https://pypi.org/pypi/{package}/{version}/json") as response:
info = json.load(response)["info"]
print(package, version)
print(*info.get("requires_dist", []), sep="\n ")
PYRepository: lnccbrown/HSSM
Length of output: 33346
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "pyproject deps:"
sed -n '1,70p' pyproject.toml
echo
echo "onnx2jax imports and jaxonnxruntime usage:"
sed -n '35,50p' src/hssm/distribution_utils/onnx_utils/onnx2jax.py
rg -n 'def make_jax_func|call_onnx|jaxonnxruntime|onnx2jax' src tests pyproject.toml || true
echo
echo "lock/env files:"
git ls-files | rg '(^|/)(uv|poetry|pip).*lock|requirements.*\.txt|conda-lock|environment\.yml|pyproject\.toml$'Repository: lnccbrown/HSSM
Length of output: 4593
Verify the JAX and jaxonnxruntime compatibility contract.
jaxonnxruntime is still imported by src/hssm/distribution_utils/onnx_utils/onnx2jax.py, so the dependency is not stale. Its 0.3.0 release predates the jax>=0.7.0 requirement and does not declare a tested compatible jax range, so this import path needs to be tested with the resolved versions or constrained to a tested JAX range.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` around lines 38 - 39, Validate the dependency pairing
represented by the jax and jaxonnxruntime entries in pyproject.toml: test the
onnx2jax.py import path with the resolved versions, or constrain jax to a range
verified compatible with jaxonnxruntime>=0.3.0. Preserve the jaxonnxruntime
dependency because it remains actively imported.
|
This is mostly a duplicate of #1137. Also declaring versions of transitive dependencies transfers the burden to us for managing the versions of these dependencies |
|
You're right on both counts — closing this in favor of #1137. On the duplicate half: confirmed. The two PRs share exactly one line of intent (capping numpy below 2.5); everything else is disjoint. Since #1137 predates this and is the maintainer's own, it should own the fix. I also re-verified the root cause and my attribution in #1144 was wrong — it is numba, as you said, not pytensor. Full traceback and the resolver bisect are in #1144; the short version is that bayesflow 2.0.13 (notebook group, never installed in CI) raised its numpy floor to On the transitive-burden half: that's a fair call and it's yours to make. I'd only note the distinction I was actually reaching for — these six aren't transitive in the "we don't use them" sense; they're imported directly by our own source (numpy in 38 files, pytensor 17, jax 17, pandas 15, pymc 12, xarray 7, arviz 1). But your point about the cost stands regardless: every floor we declare is one we then own, and bambi already pins pymc/pytensor for us. Delegating to bambi is a defensible policy, and I'm not going to relitigate it in a PR. If it's ever worth revisiting, the narrow version would be declaring only numpy — the one with no upstream guardian and the heaviest direct use — and leaving the rest to bambi. Two things I found while verifying that are worth carrying over to #1137, so they don't get lost when this closes. I've left the detail there:
|
2 participants
Closes #1144
src/hssmimports directly but never declared (arviz,jax,numpy,pandas,pymc,pytensor,xarray) — until now their versions arrived transitively via bambi/numpyro with no floor and no cap, which is exactly how the numpy 2.5 breakage landed unbounded.numpy>=2.0,<2.5cap: numpy 2.5 removednp.row_stack, which the freshly-resolved Linux CI stack (pytensor 3.2.4 path) still calls, failingtests/addm/test_addm_builder.pyon every fresh resolve of unchanged main (drift: numpy 2.5 broke the fast suite on fresh resolves (row_stack removed) #1144). The cap comment inpyproject.tomllinks the issue; once this merges, the spine's package radar tracks the cap as a standingceiling-crossedevent, so lifting it when upstream is fixed cannot be forgotten.Commands run:
uv lockresolves cleanly (288 packages; numpy → 2.4.6, pymc 6.2.0 / pytensor 3.2.4 unchanged from the last green run — lockfile not committed, HSSM resolves fresh by design).First mechanical healing PR of the drift-homeostasis rollout (see #1143); rebasing/rerunning #1143 after this merges should turn its
run_testslegs green.🤖 Generated with Claude Code
Summary by CodeRabbit