Feature/bounds to all deps#331
Conversation
WalkthroughUpdated dependency constraints in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ 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). (4)
🔇 Additional comments (5)
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
🧹 Nitpick comments (3)
pyproject.toml (3)
57-61: Dash 3 stack: validate cross‑compatibility.Dash (>=3,<4) with dash‑cytoscape (<2), dash‑daq (<1), networkx (<4), werkzeug (<4) looks coherent, but these packages have historically had tight coupling. Please validate versions resolve cleanly on Python 3.10–3.13 across Linux, macOS (x86_64/arm64), Windows.
I can provide a script to probe PyPI for wheels and a dry‑run resolver check if helpful.
67-75: Duplicate caps across “full”: keep a single source of truth if possible.Repeating the Dash/network‑viz caps in multiple extras is error‑prone. If feasible, factor them via a shared constraints file (constraints.txt) referenced by your CI/dev flows.
97-106: Docs caps look sane; add a docs build job to CI to catch future breakage.Given many mkdocs plugins are pre‑1.0 and fast‑moving, a dedicated “docs‑build” job (cache deps; pin via constraints) will surface incompatibilities early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(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). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
🔇 Additional comments (4)
pyproject.toml (4)
44-45: LGTM: sensible caps.Rich and tomli constraints look consistent with supported Python versions; marker for tomli matches 3.10 only.
47-47: LGTM: highspy major cap.Major-version ceiling avoids future API surprises while permitting 1.x bugfixes.
50-51: No action — Plotly v6 requires nbformat >= 4.2.0 (not >= 6); current pins are compatible.
Dev's "nbformat >=4.2.0, <6" is compatible with "plotly >=5.15.0, <7"; leave pyproject.toml unchanged.
79-92: Verify ruff/pre-commit minimums on PyPI — local verification required
- Ran the supplied verification script; it failed with an SSL certificate verification error in the sandbox, so I couldn't confirm PyPI availability or wheel support for "ruff >=0.9.0, <1" and "pre-commit >=4.0.0, <5" (pyproject.toml lines 79–92).
- Action: run the script locally or check ruff and pre-commit on PyPI (and wheel support for CPython 3.10–3.13) and confirm these mins are required by your configs (.pre-commit-config.yaml, pyproject/ruff config). If not required, lower the minimums to reduce contributor churn.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pyproject.toml (1)
41-41: Ultra‑narrow Linopy pin risks resolver dead‑ends.This repeats a past concern and still applies here. Either link the regression you’re avoiding or relax to the next minor as suggested.
🧹 Nitpick comments (8)
pyproject.toml (8)
51-53: Viz caps LGTM; small consistency nit on spacing.Ranges are sensible. Consider normalizing spaces around operators across the file for readability (e.g.,
< 7vs<7).Apply:
- "plotly >=5.15.0, <7", + "plotly >= 5.15.0, < 7",
58-63: Dash/Werkzeug caps: consider also bounding Flask to avoid transitive major bumps.Dash pulls in Flask; when Flask 4.0 lands, your current caps won’t prevent a resolver from selecting it unless you cap it explicitly. Suggest adding a conservative upper bound to Flask to reduce surprise breakage.
Apply:
network-viz = [ "dash >= 3.0.0, < 4", "dash-cytoscape >= 1.0.0, < 2", "dash-daq >= 0.6.0, < 1", "networkx >= 3.0.0, < 4", "werkzeug >= 3.0.0, < 4", + "Flask >= 3.0.0, < 4", ]If you’ve validated Dash 3 works with Flask 4, replace the cap with a comment explaining the tested window.
80-93: Dev pins: clarify nbformat comment and future‑proof ruff.
- The note “Related to failing CI with Plotly>6” on nbformat is confusing; nbformat isn’t directly tied to Plotly. Either link the failing CI job or clarify the real constraint.
- ruff
< 1may block adoption of 1.x when it lands without clear benefit. Consider< 2unless you know a breaking change in 1.x will hit you.Apply:
- "nbformat>=4.2.0, < 6", # Related to failing CI with Plotly>6 + "nbformat >= 4.2.0, < 6", # Pinned due to CI failure in <link-to-job or issue>; revisit quarterly - "ruff >= 0.9.0, < 1", + "ruff >= 0.9.0, < 2", # Allow 1.x; pin tighter if a specific regression appears
98-107: Docs stack: add a “latest allowed” CI canary to detect upstream breaks early.With many upper bounds, breakage will be caught late unless you exercise the top of each range. Add a scheduled job that installs docs with “--upgrade” and builds the site.
Example:
#!/bin/bash # CI canary (docs) python -m pip install --upgrade pip python -m pip install ".[docs]" mkdocs build --strict
35-53: Strategy note: upper‑bounds everywhere reduce risk but increase churn; consider constraints + weekly canary.
- Keep upper bounds, but centralize them in constraints files (e.g., constraints/core.txt, constraints/dev.txt) and have pyproject keep only minimal ranges. Use pip’s -c to enforce.
- Add a weekly “upgrade‑canary” matrix over py{3.10,3.11,3.12,3.13} to install with --upgrade within your caps for extras {core,network-viz,full,dev,docs}.
If helpful, I can PR the GitHub Actions workflow and constraints layout.
Also applies to: 58-63, 66-76, 80-93
39-53: Style consistency: normalize version spec spacing.Minor readability pass to standardize spaces around operators across all entries in these blocks.
Example:
- "highspy >= 1.5.3, < 2", + "highspy >= 1.5.3, < 2", - "linopy >=0.5.7, <0.5.8", + "linopy >= 0.5.7, < 0.6",
39-53: Relax xarray upper bound; widen Linopy pinFile: pyproject.toml (lines 39–53)
- xarray uses CalVer — allow the remainder of 2025 by using < 2026.0 instead of the very specific < 2025.9.0.
- Linopy is hard‑pinned to a single patch; widen to the next minor (< 0.6) unless you are intentionally shielding a known regression (if so, keep the pin and link the upstream issue).
Apply:
- "xarray >= 2024.2.0, < 2025.9.0", - "linopy >=0.5.7, <0.5.8", + # xarray: CalVer. Allow through the next calendar year; tighten if upstream breaks. + "xarray >= 2024.2.0, < 2026.0", + # Linopy: bound to <0.6; tighten only if a specific upstream regression is confirmed (link it). + "linopy >= 0.5.7, < 0.6",
66-76: No drift detected —fullmatchesnetwork‑vizDash stack; CI guard optional.
Ran the tomllib check: Missing in full: [], Unexpected in full: []. Recommend optionally adding the provided tomllib-based CI check or a short comment in pyproject.toml to keep extras in sync.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(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). (2)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
🔇 Additional comments (2)
pyproject.toml (2)
68-70: SciPy >= 1.15.1 floor may be unnecessarily high for manylinux availability; ensure wheel coverage.Given the comment about conflicts with highspy on earlier SciPy, the floor makes sense. Please confirm wheels exist for all supported Python versions (3.10–3.13) and common platforms to avoid source builds.
45-49: Approve — tomli marker correct; highspy cp313 wheels available (no change required).
- tomli marker (python_version < '3.11') is correct.
- highspy: cp313 wheels exist across macOS (arm64/x86_64), manylinux/musllinux and Windows for multiple releases, so the ">=1.5.3, < 2" constraint is safe for Python 3.13.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
41-41: Ultra‑narrow Linopy cap risks resolver dead‑ends.Same concern as before: pinning
<0.5.8effectively locks to a single patch. Widen unless a specific regression is documented.Proposed:
- "linopy >=0.5.1, <0.5.8", + # Linopy: widen upper bound; tighten only if a confirmed upstream regression (link it) + "linopy >= 0.5.1, < 0.6",
🧹 Nitpick comments (4)
pyproject.toml (4)
39-39: Use CalVer‑friendly cap for xarray to avoid surprise lockouts.
< 2025.9.0is brittle with xarray’s calendar versioning; prefer next‑year guard.Apply:
- "xarray >= 2024.2.0, < 2025.9.0", + # xarray uses CalVer (YYYY.MM.*); cap to next year to allow monthly patches + "xarray >= 2024.2.0, < 2026.0.0",
45-45: Upper bound on rich: add rationale or align to next major.Minor nit: consider documenting why
< 15is chosen, or simply cap to< 14if you intend “current major only.”Optional:
- "rich >= 13.0.0, < 15", + "rich >= 13.0.0, < 14", # Cap to next major; relax after CI green on 14
58-63: Avoid version drift across duplicated extras.The dash/networkx/werkzeug constraints are duplicated in
network-viz,full, anddev. This drifts over time.Consider centralizing constraints via generated constraints files (e.g.,
constraints/network-viz.txt,constraints/full.txt) and having CI validatepip install -c ... .[extra]for 3.10–3.13. I can add a small CI job to compile and check these.Also applies to: 68-76, 80-93
35-53: General: add an constraints/ADR note for “upper‑bounds everywhere” policy.Great move for stability; please document the policy (why caps, how to bump, who approves) to avoid accidental loosening later.
I can draft an ADR and a “dependency bump” checklist if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(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). (4)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
🔇 Additional comments (3)
pyproject.toml (3)
46-46: tomli conditional looks correct.
48-48: Good: add upper bound to highspy.This prevents unexpected major‑version breaks.
98-106: Docs CI: verify builds on Python 3.10–3.13 and fix mkdocs CLI invocationRan your script on Python 3.11.2: docs extras installed and resolved mkdocs 1.6.1 + plugins, but the mkdocs CLI failed (mkdocs: command not found) because pip put scripts in ~/.local/bin which isn't on PATH. Actions:
- Run the provided install+smoke job for each Python (3.10, 3.11, 3.12, 3.13).
- Make the smoke test robust: use python -m mkdocs --version (and python -m mkdocs build) or ensure CI adds $HOME/.local/bin to PATH (export PATH="$HOME/.local/bin:$PATH") / use an isolated venv so the mkdocs CLI is available.
- If any interpreter fails due to the mkdocs/plugin version caps in pyproject.toml, relax or pin the offending bounds and re-run.
| "matplotlib >= 3.5.2, < 4", | ||
| "plotly >=5.15.0, <7", | ||
| ] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Plotly range conflicts with the nbformat cap; choose one strategy.
You allow Plotly < 7 but dev caps nbformat < 6 with a comment about “failing CI with Plotly>6”. That’s contradictory and likely to resurface CI issues.
Two options:
- Option A (safer now): cap Plotly to
< 6across all extras until the break is fixed. - Option B (forward): keep Plotly
< 7, removenbformat < 6and fix whatever breaks in CI.
If you choose A, apply:
- "plotly >=5.15.0, <7",
+ # Plotly 6+ currently breaks CI; widen after fix
+ "plotly >= 5.15.0, < 6",
@@
- "dash >= 3.0.0, < 4", # Visualizing FlowSystem Network as app
+ "dash >= 3.0.0, < 4", # Visualizing FlowSystem Network as app
@@
- "dash >= 3.0.0, < 4",
+ "dash >= 3.0.0, < 4",If you choose B, remove nbformat < 6 in dev and update tests accordingly.
Please confirm which direction you prefer; I can prep a follow‑up PR accordingly.
Also applies to: 80-82, 71-71, 88-88
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Finding pyproject.toml files =="
rg --files -g 'pyproject.toml' || true
echo
echo "== Dumping pyproject.toml (first 400 lines) =="
for f in $(rg --files -g 'pyproject.toml' || true); do
echo "---- $f ----"
nl -ba "$f" | sed -n '1,400p' || true
echo
done
echo "== Searching repository for 'nbformat' and 'plotly' =="
rg -n --hidden --glob '!**/.git/**' -S -C3 'nbformat|plotly' || true
echo
echo "== Searching for an explicit 'nbformat < 6' cap or comment about Plotly>6 =="
rg -n --hidden --glob '!**/.git/**' -S 'nbformat\s*<\s*6|Plotly\s*>\s*6|failing CI with Plotly' || true
echo
echo "== Searching .github workflows for mentions =="
rg -n --hidden -S -g '.github/**' 'plotly|nbformat' || trueLength of output: 644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pyproject.toml exists? =="
if [ -f pyproject.toml ]; then
echo "present"
else
echo "missing"
fi
echo
echo "== pyproject.toml (numbered, first 200 lines) =="
if [ -f pyproject.toml ]; then
awk 'NR<=200 {printf "%4d: %s\n", NR, $0}' pyproject.toml
fi
echo
echo "== pyproject.toml lines 45-95 =="
if [ -f pyproject.toml ]; then
awk 'NR>=45 && NR<=95 {printf "%4d: %s\n", NR, $0}' pyproject.toml
fi
echo
echo "== Search for 'plotly' or 'nbformat' in repo (no ignore) =="
rg -n --no-ignore -S 'plotly|nbformat' || true
echo
echo "== Search for explicit 'nbformat < 6' in pyproject.toml =="
rg -n --no-ignore -S 'nbformat\s*<\s*6' pyproject.toml || true
echo
echo "== Search .github workflows for plotly/nbformat (if present) =="
rg -n --no-ignore -S 'plotly|nbformat' .github || true
echo "== done =="Length of output: 17676
Plotly range conflicts with dev nbformat cap — choose A or B
pyproject.toml: plotly >=5.15.0, <7 (lines 51–53) vs dev nbformat>=4.2.0, <6 (lines 80–82) — dev comment: "Related to failing CI with Plotly>6".
- Option A (safer): pin Plotly to <6 across the project. Apply:
- "plotly >=5.15.0, <7",
+ # Plotly 6+ currently breaks CI; widen after fix
+ "plotly >= 5.15.0, < 6",- Option B (forward): keep Plotly <7, remove the nbformat <6 cap from dev extras and fix the CI/tests that depend on nbformat compatibility.
Confirm which direction to take; I will prepare the follow‑up PR.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "matplotlib >= 3.5.2, < 4", | |
| "plotly >=5.15.0, <7", | |
| ] | |
| "matplotlib >= 3.5.2, < 4", | |
| # Plotly 6+ currently breaks CI; widen after fix | |
| "plotly >= 5.15.0, < 6", | |
| ] |
🤖 Prompt for AI Agents
In pyproject.toml around lines 51–53, the Plotly version range (>=5.15.0, <7)
conflicts with the dev constraint on nbformat (<6) which CI indicates fails for
Plotly >6; choose the safer Option A: pin Plotly to <6 across the project. Edit
pyproject.toml to change the plotly requirement to "plotly >=5.15.0, <6", search
for and update any other dependency declarations or lockfile entries that
reference Plotly to the new cap, run the test suite and CI locally to confirm
the conflict is resolved, and update the changelog or dependency notes to
document the version pin and rationale.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
52-52: Plotly vs nbformat constraint conflict still unresolved — pick A or B.You allow Plotly < 7, but keep nbformat < 6 citing CI failures with Plotly>6. This contradiction will resurface. Choose one:
- Option A (safer now): cap Plotly to < 6 across extras.
- Option B (forward): keep Plotly < 7 and remove nbformat < 6, then fix CI.
If you choose A, apply:
- "plotly >= 5.15.0, < 7", + # Plotly 6+ currently breaks CI; widen after fix + "plotly >= 5.15.0, < 6",If you choose B, drop
nbformat < 6and adjust tests/CI accordingly.Also applies to: 82-82
🧹 Nitpick comments (3)
pyproject.toml (3)
37-39: Align NumPy floor with consumers (SciPy/xarray) and Python 3.13.Base allows NumPy >= 1.21.5, but SciPy 1.15.x and newer xarray releases often require newer NumPy (and wheels) especially on Py 3.13. Avoid impossible combos.
Option 1 (simplest): raise base NumPy to 2.x to match SciPy in extras.
- "numpy >= 1.21.5, < 3", + "numpy >= 2.0.0, < 3",Option 2 (granular): use environment‑specific floors if you want to keep 1.x for Py<=3.11.
- "numpy >= 1.21.5, < 3", + "numpy >= 1.24, < 3; python_version < '3.12'", + "numpy >= 2.0, < 3; python_version >= '3.12'",Please confirm your CI includes 3.10–3.13 to catch wheel availability issues (notably highspy/scipy).
Also applies to: 70-70, 87-87
51-64: Consistent rationale comments for all tight upper bounds.Nice use of comments for several pins; add brief rationale/issue links for the rest (matplotlib, dash stack, docs plugins) to aid future maintainers.
Also applies to: 68-77, 99-108
35-53: Consider adopting a constraints/lock workflow for bounds stability.Given many new upper bounds, use pip‑tools/uv constraints to validate ranges and regularly refresh, plus a CI “pip check” across extras to detect conflicts early.
I can provide a minimal constraints.txt flow and a GitHub Actions job to exercise each extras group.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(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). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (2)
pyproject.toml (2)
41-41: Good fix: widened Linopy cap from a patch pin to a minor range.This removes the resolver dead‑end risk while keeping you safe from 0.6 breaking changes.
Please confirm that the minimum 0.5.1 still contains all APIs you rely on (previous lower bound was 0.5.7). If not, bump the floor to the earliest known‑good.
48-48: Solver wheels on Python 3.13: verify before advertising support.highspy and gurobipy often lag on newest Python wheels. Since classifiers include Python 3.13, confirm wheels exist; otherwise gate 3.13 in classifiers/CI until upstream publishes.
I can add a CI job that probes PyPI wheels for these packages across 3.10–3.13 without building from source.
Also applies to: 71-71
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
pyproject.toml (4)
37-38: Add Python‑version‑aware minimums for Py3.12/3.13 compatibility.Floors like numpy>=1.21.5 and matplotlib>=3.5.2 may be too low for Py3.12/3.13. Raise floors conditionally to avoid resolver churn/build failures on newer Pythons.
Example (verify exact floors before committing):
"numpy >= 1.21.5, < 3", +"numpy >= 1.26.0; python_version >= '3.12'", @@ "matplotlib >= 3.5.2, < 4", +"matplotlib >= 3.8; python_version >= '3.12'",Also applies to: 51-51
44-44: Normalize package name casing for consistency.PEP 503 makes names case‑insensitive, but we keep them lowercase for consistency.
- "PyYAML >= 6.0.0, < 7", + "pyyaml >= 6.0.0, < 7", @@ - "Flask >= 3.0.0, < 4", # Explicit Flask cap to prevent transitive major bumps + "flask >= 3.0.0, < 4", # Explicit Flask cap to prevent transitive major bumpsAlso applies to: 63-63
81-94: Avoid hard pins in the dev extra; prefer a constraints/lock for reproducibility.Pinning with == in extras can cause unnecessary conflicts for contributors. Keep ranges here and maintain exact pins in a constraints file (e.g., constraints-dev.txt) used by CI.
If you want, I can prepare a follow‑up PR that introduces constraints files and updates CI install commands.
25-25: Duplicate license classifier entry.You list “MIT License” twice in classifiers; drop one to reduce noise.
"License :: OSI Approved :: MIT License", @@ -"License :: OSI Approved :: MIT License",Also applies to: 33-33
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(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: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
🔇 Additional comments (4)
pyproject.toml (4)
41-41: Good fix: widened Linopy pin avoids resolver dead‑ends.Relaxing to < 0.6 resolves the prior ultra‑narrow pin concern and keeps us future‑proof within the minor.
46-46: Tomli marker looks correct.Conditional dependency for Python < 3.11 aligns with requires‑python >=3.10.
98-108: Docs stack is fully pinned; check Py3.13 wheels.Exact pins are fine for reproducible docs, but confirm all have Py3.13 wheels to keep classifiers honest.
48-48: Resolve — prebuilt highspy cp313 wheels foundPyPI publishes cp313 wheels for highspy 1.8.1, 1.9.0, 1.10.0 and 1.11.0; with "highspy >= 1.5.3, < 2" pip will be able to install a wheel on Python 3.13, so advertising Py3.13 support is OK. (pyproject.toml: line 48)
| "matplotlib >= 3.5.2, < 4.0.0", | ||
| "plotly >=5.15.0, <6.4.0", | ||
| "matplotlib >= 3.5.2, < 4", | ||
| "plotly >= 5.15.0, < 7", |
There was a problem hiding this comment.
❓ Verification inconclusive
Resolve Plotly↔nbformat mismatch to prevent CI breakage.
Base allows Plotly < 7 while dev pins nbformat==5.10.4 (<6). If Plotly 6.x needs nbformat≥6 (or vice‑versa), installs with the dev extra will fail. Choose one strategy:
- Option A (safer now): cap Plotly to < 6 across base until CI is fixed.
- Option B (forward): keep Plotly < 7, drop/raise the nbformat cap, and fix CI.
Recommended (A) for this PR’s “bounds” scope:
- "plotly >= 5.15.0, < 7",
+ # Plotly 6+ conflicts with nbformat<6 in dev; widen after CI fix
+ "plotly >= 5.15.0, < 6",Also applies to: 81-82
Cap Plotly to <6 in base deps to avoid nbformat mismatch / CI breakage.
Base currently allows "plotly >= 5.15.0, < 7" while dev pins nbformat==5.10.4 (<6). Plotly v6 drops support for older Jupyter and introduced regressions that have broken notebook rendering/HTML export — pin Plotly <6 until CI and notebook integrations are validated. (plotly.com)
- "plotly >= 5.15.0, < 7",
+ # Plotly 6+ conflicts with nbformat<6 in dev; widen after CI fix
+ "plotly >= 5.15.0, < 6",Also applies to: 81-82
🤖 Prompt for AI Agents
In pyproject.toml around line 52 (and also update occurrences at lines 81-82),
the Plotly base dependency range allows "< 7" which can pull Plotly v6 and cause
nbformat/Jupyter/CI breakage; change the constraint to cap Plotly to "< 6"
(e.g., "plotly >= 5.15.0, < 6") in the base deps and make the same change for
the other listed occurrences so CI and notebook integrations keep using the
tested v5 series.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
52-52: Resolve Plotly<7 vs nbformat==5.10.4 mismatch (repeat from earlier review).Base allows Plotly <7 while dev pins nbformat to 5.x. This is the same issue flagged earlier; please choose a direction (cap Plotly to <6 now or relax/remove the nbformat<6 constraint and fix CI). Safer for this PR: cap Plotly to <6.
- "plotly >= 5.15.0, < 7", + # Plotly 6+ conflicts with nbformat==5.10.4 in dev; widen after CI fix + "plotly >= 5.15.0, < 6",Also applies to: 81-82
🧹 Nitpick comments (3)
pyproject.toml (3)
57-57: Prefer underscore in extras key names.Hyphens in extras work but are easy to mistype;
pip install .[network-viz]vs normalization to underscores. Rename tonetwork_vizfor consistency.-[project.optional-dependencies] -# Interactive network visualization with enhanced color picker -network-viz = [ +[project.optional-dependencies] +# Interactive network visualization with enhanced color picker +network_viz = [
44-44: Normalize package names to lowercase for consistency.PEP 503 normalizes names; use lowercase for consistency in specs.
- "PyYAML >= 6.0.0, < 7", + "pyyaml >= 6.0.0, < 7", @@ - "Flask >= 3.0.0, < 4", # Explicit Flask cap to prevent transitive major bumps + "flask >= 3.0.0, < 4", # Explicit Flask cap to prevent transitive major bumpsAlso applies to: 63-63
68-68: Unify requirement formatting.Minor nit: keep spacing style consistent around
==.- "pyvis ==0.3.2", # Visualizing FlowSystem Network + "pyvis==0.3.2", # Visualizing FlowSystem NetworkAlso applies to: 85-85
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(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). (4)
- GitHub Check: test (3.10)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (1)
pyproject.toml (1)
39-39: Verify Python 3.13 support vs lower bounds; add python_version-specific floors if needed.xarray 2024.x, pandas 2.x, matplotlib 3.5.x, and numpy min 1.21.5 may not all ship wheels or be compatible on 3.13. Either confirm with CI across 3.10–3.13 or raise per‑Python floors via environment markers.
Suggested checks (run locally/CI): pip install .[full] and .[dev] on 3.10–3.13; if 3.13 fails, raise numpy/pandas/matplotlib mins for 3.13 only using markers like
; python_version >= '3.13'.Also applies to: 37-38, 51-52
| "werkzeug >= 3.0.0", # Visualizing FlowSystem Network as app | ||
| "tsam >= 2.3.1, < 3", # Time series aggregation | ||
| "scipy >= 1.15.1, < 2", # Used by tsam. Prior versions have conflict with highspy. See https://github.com/scipy/scipy/issues/22257 | ||
| "gurobipy >= 10.0.0, < 13", |
There was a problem hiding this comment.
Don't force proprietary solver into the dev extra.
Pinning gurobipy in [dev] makes contributor installs fail without a license. Keep gurobi in feature extras (e.g., full/solvers) and remove it from dev, or gate it behind a separate [dev-solver] extra.
dev = [
@@
- "gurobipy==12.0.3",
]Optionally add:
+[project.optional-dependencies]
+dev-solver = [
+ "gurobipy==12.0.3",
+]Also applies to: 88-88
🤖 Prompt for AI Agents
In pyproject.toml around line 71 (and also line 88), the proprietary package
"gurobipy >= 10.0.0, < 13" is incorrectly listed in the [dev] extras causing
contributor installs to fail without a license; remove gurobipy from the [dev]
extra and instead add it to a feature extra such as
[project.optional-dependencies.full] or [project.optional-dependencies.solvers],
or create a separate [project.optional-dependencies."dev-solver"] extra (and
update any docs/CI that reference the dev extra) so that installing dev
dependencies no longer requires a gurobi license.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
pyproject.toml (3)
41-41: Linopy range widening looks good.Relaxing to
< 0.6reduces resolver dead-ends while keeping a safe upper bound.
52-52: Resolve Plotly↔nbformat mismatch (choose A or B).Base allows Plotly <7 while dev pins nbformat==5.10.4 (<6). To avoid CI/env conflicts:
- Option A (safer now): cap Plotly to <6.
- Option B (forward): keep Plotly <7 and raise/remove the nbformat<6 pin in dev, then fix CI.
- "plotly >= 5.15.0, < 7", + # Plotly 6+ conflicts with dev nbformat<6; widen after CI fix + "plotly >= 5.15.0, < 6",
81-94: Don’t require proprietary solver in dev extra.
gurobipyin[dev]blocks contributors without a license. Remove fromdev; if needed, add a separatedev-solverextra.dev = [ @@ - "gurobipy==12.0.3", @@ ] + +[project.optional-dependencies] +# (append to existing table) +dev-solver = [ + "gurobipy==12.0.3", +]
🧹 Nitpick comments (4)
flixopt/flow_system.py (2)
247-249: Docstring: list all required optional deps and fix package name.Mention
dash-daqandflask, and use the correct package namedash-cytoscape(hyphen, not underscore).- """Visualizes the network structure of a FlowSystem using Dash, Cytoscape, and networkx. - Requires optional dependencies: dash, dash-cytoscape, networkx, werkzeug. + """Visualizes the network structure of a FlowSystem using Dash, Cytoscape, and networkx. + Requires optional dependencies: dash, dash-cytoscape, dash-daq, networkx, flask, werkzeug. """
278-283: Unify stop() guidance with start(); offer full extra too.Match the start() message and include the full extra.
- f'Install with: pip install flixopt[network_viz]. ' + f'Install with: pip install flixopt[network_viz] or flixopt[full]. 'pyproject.toml (2)
68-77: Full extra should include Flask to match network_viz caps.To keep constraints consistent and avoid transitive-major surprises, add Flask here too.
"networkx >= 3.0.0, < 4", # Visualizing FlowSystem Network as app "werkzeug >= 3.0.0, < 4", # Visualizing FlowSystem Network as app + "flask >= 3.0.0, < 4", # Keep in sync with network_viz extra
81-94: Pin Flask in dev for reproducible env (align with dash/werkzeug pins).Dev pins Dash/Werkzeug but not Flask. Add Flask to avoid transitive drift.
"werkzeug==3.0.0", + "flask==3.0.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
flixopt/flow_system.py(2 hunks)pyproject.toml(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). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
🔇 Additional comments (1)
pyproject.toml (1)
57-64: Rename to network_viz: LGTM; sweep repo for old references.The extra name matches code changes. Please update docs/examples that still use
[viz]ornetwork-viz.#!/bin/bash # Find old extra names/usages to update rg -nP "flixopt\\[(viz|network-viz)\\]" -S || true rg -nP "pip install.+dash[_-]cytoscape" -S || true
| if not DASH_CYTOSCAPE_AVAILABLE: | ||
| raise ImportError( | ||
| f'Network visualization requires optional dependencies. ' | ||
| f'Install with: pip install flixopt[viz], flixopt[full] or pip install dash dash_cytoscape networkx werkzeug. ' | ||
| f'Install with: pip install flixopt[network_viz], flixopt[full] or pip install dash dash_cytoscape networkx werkzeug. ' | ||
| f'Original error: {VISUALIZATION_ERROR}' | ||
| ) |
There was a problem hiding this comment.
Fix pip package name and include all extras in the Install hint.
dash_cytoscapeshould bedash-cytoscape.- Include
dash-daqandflask. - Keep
flixopt[full]as an alternative.
- f'Install with: pip install flixopt[network_viz], flixopt[full] or pip install dash dash_cytoscape networkx werkzeug. '
+ f'Install with: pip install flixopt[network_viz] or flixopt[full], '
+ f'or: pip install dash dash-cytoscape dash-daq networkx flask werkzeug. '📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not DASH_CYTOSCAPE_AVAILABLE: | |
| raise ImportError( | |
| f'Network visualization requires optional dependencies. ' | |
| f'Install with: pip install flixopt[viz], flixopt[full] or pip install dash dash_cytoscape networkx werkzeug. ' | |
| f'Install with: pip install flixopt[network_viz], flixopt[full] or pip install dash dash_cytoscape networkx werkzeug. ' | |
| f'Original error: {VISUALIZATION_ERROR}' | |
| ) | |
| if not DASH_CYTOSCAPE_AVAILABLE: | |
| raise ImportError( | |
| f'Network visualization requires optional dependencies. ' | |
| f'Install with: pip install flixopt[network_viz] or flixopt[full], ' | |
| f'or: pip install dash dash-cytoscape dash-daq networkx flask werkzeug. ' | |
| f'Original error: {VISUALIZATION_ERROR}' | |
| ) |
🤖 Prompt for AI Agents
In flixopt/flow_system.py around lines 258 to 263, the install hint text is
wrong and incomplete: change the package name from dash_cytoscape to
dash-cytoscape, add dash-daq and flask to the list, and present flixopt[full] as
an alternative; update the message to something like "Install with: pip install
flixopt[network_viz,full] or pip install dash dash-cytoscape dash-daq networkx
werkzeug flask" and include the original error variable as before.
1 participant
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
Chores
Documentation
Bug Fixes
Style