Change 3D plotting from MayaVi to PyVista - #33
Conversation
Summary of Changes(Generated by Gemini 3.6 Flash (High) via Google Antigravity) This pull request completes the migration of the PyEQSP 3D visualization backend from legacy MayaVi ( 1. Visualization Backend Modernization (
|
There was a problem hiding this comment.
🟡 Changes recommended
The current changes introduce mismatches/bugs (notably around title handling and mock/doctest stability via window_size, plus an incorrect install hint) that should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR migrates PyEQSP’s 3D visualization backend from MayaVi to PyVista (per #32), updating the implementation, test mocks/doctests, examples, and documentation, and bumping the package version to 1.0b3.
Changes:
- Replaced MayaVi-based plotting in
eqsp.visualizationswith PyVistaPlotter-based rendering and screenshot support. - Refactored visualization mock tests and coverage/doctest tooling to work with PyVista and off-screen rendering.
- Updated installation guidance, user/maintainer docs, and examples to reference PyVista and the new optional extra.
File summaries
| File | Description |
|---|---|
| tests/src/test_visualizations_mock.py | Updates mock-based visualization tests from MayaVi to PyVista. |
| tests/src/inspect_visualizations.py | Updates manual inspection helper to use PyVista and off-screen mode. |
| tests/run_coverage.py | Skips visualization doctests/coverage when PyVista is unavailable. |
| README.md | Updates release metadata and switches 3D backend docs to PyVista. |
| pyproject.toml | Bumps version to 1.0b3 and replaces mayavi extra with pyvista. |
| INSTALL.md | Updates installation instructions and extras name to PyVista. |
| examples/user-guide/src/example_visualize_3d.py | Updates 3D example script messaging and dependency notes to PyVista. |
| examples/user-guide/README.md | Updates user-guide instructions for installing the PyVista extra. |
| examples/phd-thesis/src/fig_4_10_eqp_voronoi_s2_33.py | Ports thesis figure script from MayaVi to PyVista (tubes/meshes + screenshots). |
| examples/phd-thesis/src/fig_4_1_eqp_s2_33.py | Ports thesis figure script from MayaVi to PyVista and uses screenshots. |
| examples/phd-thesis/src/fig_3_1_partition_s2_33.py | Ports thesis figure script from MayaVi to PyVista and uses screenshots. |
| examples/phd-thesis/regenerate_figures.py | Updates regeneration harness messaging and backend detection to PyVista. |
| eqsp/visualizations.py | Core backend migration from MayaVi to PyVista; adds plotter plumbing and screenshots. |
| doc/user/visualization_guide.md | Updates user visualization guide from MayaVi to PyVista guidance. |
| doc/user/phd-thesis-examples.md | Updates thesis reproduction notes and headless rendering environment variables for PyVista. |
| doc/user/migration_matlab.md | Updates MATLAB migration doc to reflect PyVista as the optional 3D backend. |
| doc/user/installation.md | Updates installation/setup guidance to PyVista and off-screen rendering. |
| doc/user_guide.md | Updates high-level user guide references to PyVista for 3D rendering. |
| doc/maintainer/testing_details.md | Updates maintainer testing docs to refer to PyVista in mock tests. |
| doc/maintainer/technical_symmetry.md | Updates release header references to 1.0b3. |
| doc/maintainer/release_roadmap.md | Adds 1.0b3 roadmap entry describing PyVista backend migration. |
| doc/maintainer/release_notes.md | Adds 1.0b3 release notes describing the PyVista migration. |
| doc/maintainer/documentation_maintenance.md | Updates maintenance guidance wording around 3D engine transitions. |
| doc/maintainer/design_and_architecture.md | Updates architecture docs/diagram to reference PyVista instead of MayaVi. |
| doc/conf.py | Updates Sphinx mocking/headless handling to use PyVista instead of MayaVi/PyQt5. |
| CONTRIBUTING.md | Updates release header references to 1.0b3. |
| CHANGELOG.md | Adds 1.0b3 changelog entry for PyVista migration and related updates. |
| AUTHORS.md | Updates release header references to 1.0b3. |
Review details
Suppressed comments (1)
tests/src/test_visualizations_mock.py:38
- The visualization code uses
pl.window_size[...]when adding titles. Withpyvistamocked,Plotter().window_sizeis currently aMagicMock, which will raise when used inint(title_pos[0] * pl.window_size[0]). Setting a realisticwindow_sizeon the mocked Plotter avoids unit test failures.
- Files reviewed: 28/33 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness/documentation issues (notably the Sphinx PyVista mock and silent ignored kwargs) that can break doctest-style runs or hide user errors.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
eqsp/visualizations.py:186
show_s2_partitionaccepts**_kwargsbut never reads or forwards it, which will silently ignore unexpected/legacy keyword arguments and can hide user mistakes. Either remove the variadic kwargs from the signature or explicitly reject any unexpected kwargs so callers get a clear error.
title_text = None
- Files reviewed: 28/33 changed files
- Comments generated: 2
- Review effort level: Lite
|
From Google Antigravity: Summary of PR #33 Review Resolutions & Title Parity Updates
|
Closes #32