pyopenms_viz icon indicating copy to clipboard operation
pyopenms_viz copied to clipboard

Support python 3.13

Open poshul opened this issue 4 months ago • 29 comments

Summary by CodeRabbit

  • Chores

    • Project version bumped to 1.0.1; Python support broadened and CI now tests multiple Python versions (recommended 3.13). Large dependency upgrades applied.
  • Documentation

    • Installation guidance and hosted docs updated to recommend Python 3.13 and a newer build OS.
  • Visuals / Bug Fixes

    • Snapshot data encoded more compactly; heatmapgl traces removed and a new scattermap trace type added. Bokeh frontend upgraded.
  • Tests

    • Deterministic test utilities added and snapshot comparisons relaxed for minor rendering differences.

poshul avatar Aug 11 '25 09:08 poshul

Walkthrough

Updated Python targets across CI, Binder, and docs; bumped project version and widened Python requirement; regenerated dependencies; converted many test snapshots to binary-encoded payloads and swapped heatmapgl for scattermap; upgraded Bokeh snapshots; added deterministic test fixtures; and enhanced Matplotlib/Bokeh/Plotly snapshot comparators.

Changes

Cohort / File(s) Summary
CI workflows
.github/workflows/ci.yml, .github/workflows/execute_notebooks.yml, .github/workflows/static.yml
Extended CI Python matrix to include 3.13 and 3.14 (ci.yml) and updated notebook/static workflows to use 3.13; step logic unchanged.
Binder / ReadTheDocs
.binder/runtime.txt, .readthedocs.yaml
Binder runtime changed to python-3.13; RTD OS bumped to ubuntu-24.04 and Python to 3.13.
Documentation
README.md, docs/Installation.rst
Installation examples and recommended Python version updated to 3.13.
Project metadata
pyproject.toml
Version bumped 1.0.0 → 1.0.1; added classifiers and widened requires-python to <=3.14.
Dependencies
requirements.txt
Regenerated dependency list: many additions, removals, and version bumps; header updated for Python 3.13.
Test fixtures
test/conftest.py
Added session-scoped autouse fixture to produce deterministic UUID/Bokeh IDs and per-test autouse fixture to reseed Python/NumPy RNGs and reset counters.
Snapshot comparators
pyopenms_viz/testing/MatplotlibSnapshotExtension.py, .../BokehSnapshotExtension.py, .../PlotlySnapshotExtension.py
Matplotlib: allow ≤1% pixel diffs. Bokeh: recursive compare_json with ignored keys (id, root_ids), ndarray decoding, stable sorting of dict-lists, path-aware diagnostics; signature updated. Plotly: bdata decode, array-compare helpers, tolerances, contextual errors; compare_json signature extended; new helpers added.
Plot snapshots
test/__snapshots__/**/*.json
Many Plotly snapshots converted x/y/color/customdata from inline arrays to { "dtype": ..., "bdata": "..." }; numerous heatmapgl blocks removed and scattermap traces/layout entries added.
Bokeh HTML snapshots
test/__snapshots__/*bokeh*.html
Bokeh assets upgraded to 3.7.3; embedded div IDs, docs_json script IDs and render_items mappings updated across snapshots.
Other snapshots
test/__snapshots__/**/*
Wide set of chromatogram/peakmap/spectrum/marginal/3D snapshots updated for binary encoding, trace-type changes, and Bokeh ID/version updates.

Sequence Diagram(s)

sequenceDiagram
  participant T as Test code
  participant S as Serializer (Plot/Bokeh)
  participant Snap as Snapshot (JSON/HTML)
  participant C as Comparator (Matplotlib/Bokeh/Plotly)
  Note over T,S: test creates figure
  T->>S: build & serialize figure
  alt Plotly binary encoding
    S->>Snap: emit fields as { "dtype", "bdata" } for x/y/color/customdata
  else Bokeh HTML
    S->>Snap: emit embedded docs_json, root IDs, and Bokeh version
  end
  Note over C,Snap: assertion phase
  C->>Snap: load snapshot
  alt Plotly comparator
    C->>C: decode bdata (base64 ± zlib) → ndarray
    C->>C: align/sort tuples (y ↔ customdata), compare arrays with tolerance
  else Bokeh comparator
    C->>C: recurse dict/list, ignore keys (`id`,`root_ids`), compare ndarrays/text
  else Matplotlib comparator
    C->>C: image diff, allow ≤1% differing pixels
  end
  C-->>T: test pass/fail

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Files/areas to focus on:
    • pyopenms_viz/testing/PlotlySnapshotExtension.py — base64/zlib decode, dtype handling, array alignment/sorting, and tolerance behavior.
    • pyopenms_viz/testing/BokehSnapshotExtension.py — recursive compare_json logic, ignored keys, ndarray handling, and list sorting stability.
    • test/conftest.py — safety of uuid monkeypatch, Bokeh ID counter handling, and restoration semantics.
    • Large snapshot diffs — verify serializer changes (binary encoding) and heatmapglscattermap replacements reflect intended output.
    • requirements.txt / pyproject.toml — confirm dependency bumps and Python compatibility align with CI/change.

Possibly related PRs

  • OpenMS/pyopenms_viz#39 — overlaps changes to snapshot comparison utilities and testing infrastructure; likely related to the comparator and fixture edits.

Suggested reviewers

  • singjc

Poem

I twitch my whiskers, carrot-bright,
From three-dot-twelve to three-dot-thirteen's light.
CI hops more lanes and snapshots tuck their bits,
Bokeh swapped its IDs and Plotly packed its bits.
A rabbit hums — deterministic tests and bliss! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Support python 3.13' directly and clearly describes the main objective of the changeset. The raw summary confirms that the PR systematically updates Python version references across configuration files (.binder/runtime.txt, .github/workflows/*, .readthedocs.yaml, README.md, docs/Installation.rst, pyproject.toml), CI/CD matrices, and documentation to support Python 3.13 alongside existing versions. The title accurately captures the primary change from the developer's perspective.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch poshul-patch-1

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2067db21e89280d28d9a254e1662f10366b2d5cc and 5010aa3c9b7516f096046c1d06b31fb9a2612f34.

📒 Files selected for processing (1)
  • test/conftest.py (2 hunks)
🔇 Additional comments (3)
test/conftest.py (3)

12-22: LGTM! Deterministic matplotlib configuration is appropriate.

The matplotlib settings effectively address snapshot testing issues by ensuring consistent rendering across different systems and Python versions. The explicit font, hashsalt, and path simplification settings will help prevent the visual test failures described in the PR objectives.


107-137: Previous issue resolved - deterministic ID setup looks good.

The session fixture properly patches uuid.uuid4 with a deterministic implementation and exposes the counter for per-test resets. The cleanup code (lines 135-137) now correctly restores the original uuid4 and removes the counter attribute without using a bare except clause.


140-160: Previous issue resolved - per-test ID reset implemented correctly.

The per-test fixture now resets both the RNG seeds and the deterministic ID counters before each test, ensuring that tests produce consistent snapshots regardless of execution order or which subset of tests is run. This directly addresses the previous review comment about enabling targeted test runs without full suite execution.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Aug 11 '25 09:08 coderabbitai[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: dd18d40

github-actions[bot] avatar Aug 11 '25 10:08 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: ea86dd8

github-actions[bot] avatar Aug 11 '25 11:08 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: b2cd981

github-actions[bot] avatar Aug 11 '25 11:08 github-actions[bot]

In theory this should work however, the ci tests will likely have to be updated because they are all visual tests and thus very specific to all the versions that they were created with. I am ok if you want to update 3.13 to be supported however we should keep the testing at 3.12 for now.

jcharkow avatar Aug 13 '25 13:08 jcharkow

Any update on this? I would like to see Python 3.13 supported as well.

BioGeek avatar Nov 03 '25 09:11 BioGeek

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 97d11ff

github-actions[bot] avatar Nov 03 '25 10:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: b3e6a18

github-actions[bot] avatar Nov 03 '25 10:11 github-actions[bot]

@jcharkow I think the testing issues are due to the updated bokeh, rather than the python version. I'm working on adding some code to the testing script to try to keep those consistent.

poshul avatar Nov 03 '25 11:11 poshul

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: f7dfc1f

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 6706bc7

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 449d3c2

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 923713c

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 5e85c0b

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 4f39509

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 1986648

github-actions[bot] avatar Nov 03 '25 11:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 7b3bd28

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 3a82f6c

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: e8a11ac

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 4a72bf2

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: e4535dc

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 4f0e7eb

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 7a2941f

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: f2e4e6c

github-actions[bot] avatar Nov 03 '25 12:11 github-actions[bot]

I'm out of time to fight with this. Here's the problems:

  • the main branch snapshot tests fail if UUIDs don't match. UUIDs are not generated deterministically, so on different set-ups they won't match (hence generating the snapshots on one computer and testing them on another fails).
  • Similarly the snapshot tests test for order of elements in datatypes that are unordered, so again it fails on different machines
  • One of the updated libraries now encodes some of the JSON structures in base64, the snapshot tests fail on these. I've been playing whack-a-mole with this with co-pilot, but basically the PlotlySnapshotExtension and BokehSnapshotExtension should decode base64, sort the elements, and then do the comparison.

poshul avatar Nov 03 '25 13:11 poshul

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: d5fbdcd

github-actions[bot] avatar Nov 03 '25 13:11 github-actions[bot]

📖 Documentation Preview

The documentation for this PR has been built and is available at: 🔗 View Preview

This preview will be updated automatically when you push new commits to this PR.


Preview built from commit: 419a516

github-actions[bot] avatar Nov 03 '25 13:11 github-actions[bot]

Note to self: things to look into

  • DeepDiff - https://zepworks.com/deepdiff/current/
  • Fuzzy Logic with pngs -
baseline = imagehash.average_hash(Image.open("baseline.png"))
current = imagehash.average_hash(Image.open("plot.png"))

# Returns 0 for identical, higher for different
# Typically use threshold like: if baseline - current < 5: pass
difference = baseline - current
assert difference < 5, f"Plot rendering changed too much: {difference}"

jcharkow avatar Nov 03 '25 14:11 jcharkow

Testing for visualizations is definitely annoying and tricky...

I feel like we run into issues with bokeh updates, we had to switch from v2 to v3, and seem to be running into more issues. If keeping up-to-date with bokeh is a big problem, I am wondering if we should deprecate support for bokeh, since plotly already covers the same and more (also don't get issues with running plotly in jupyter notebooks). Would also make maintaining easier with one less plotting library to worry about.

For testing, I'm not sure if pytest-snapshot is also contributing to the complexity/failing of the tests with what @poshul mentioned above. Maybe we should try a different approach, which I think @jcharkow is thinking about.

Not sure if it would be easier, but maybe we could just convert the images to numpy arrays and use pytest-regtest to capture and compare against. I think that has been more stable and reliable.

Example for matplotlib

p = df.plot(x="rt", 
                 y="im", 
                 z="int", 
                 kind="peakmap", 
                 backend="ms_matplotlib"
)

fig = p.get_figure()
fig.canvas.draw()
buf = np.array(fig.canvas.buffer_rgba())

# We would record and compare the buf array of the iamge, we can compare the shape as well buf.shape

# For internal testing to validate the np array represents the actual plot, we can plot the array
plt.figure(figsize=(8, 6))
plt.imshow(buf)
plt.title("This should match the original plot")
plt.axis('off')
plt.show()

Might be more tricky to do with plotly, would need additional testing dependencies (kaleidescope and chrome) for converting plotly images to numpy arrays. Or we can just extract the data lists from the plotly figure object, sort them and compare. :man_shrugging:

singjc avatar Nov 03 '25 16:11 singjc