Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new plotting tools – one for 1D velocity models and another for plotting realisation rupture paths – together with updated documentation and expanded test coverage.
- Added comprehensive markdown guides for using the new tools (plotting 1D velocity models and plotting realisation domains/rupture paths).
- Implemented new modules in the visualisation package (for rupture path and 1D velocity model plotting) and updated the CLI script mappings.
- Extended tests to validate the new plotting functions and ensure image outputs match expected examples.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wiki/Velocity-Models.md | New documentation for the 1D velocity model plotting tool. |
| wiki/Realisations.md | Added sections on domain plotting and rupture path plotting in realisations. |
| visualisation/utils.py | New helper functions to convert and manipulate geometry for plotting. |
| visualisation/plot_rupture_path.py | Introduces rupture path plotting using PyGMT and Typer CLI integration. |
| visualisation/plot_1d_velocity_model.py | New module for plotting 1D velocity models with customizable subplot options. |
| tests/test_plots.py | Expanded tests for various plotting functions including rupture path and velocity models. |
| pyproject.toml | Updated CLI scripts to include the new plotting tools. |
Comments suppressed due to low confidence (1)
tests/test_plots.py:319
- The rupture path test is using the 'velocity_model_plot_file' as input instead of a realisation JSON file, which is expected by the plotting function. Please update the test to supply the correct realisation file.
plot_rupture_path.plot_rupture_path_to_file(velocity_model_plot_file, output_image_path,
|
Updated for the new testing code |
There was a problem hiding this comment.
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tests/test_plots.py:354
- [nitpick] The parameter name 'velocity_model_plot_file' is ambiguous in the context of test_plot_rupture_path. Consider renaming it to 'realisation_file' for clarity.
def test_plot_rupture_path(velocity_model_plot_file: Path, output_image_path: Path, plot_image_dir: Path):
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new CLI tool for plotting rupture paths from realisations without SRFs, providing a helpful debugging and exploratory utility.
- Adds documentation updates in the project wiki for the new tool.
- Includes new functions in the visualisation module to plot rupture paths and updates the source plotting by accepting additional kwargs.
- Updates tests and CLI command mapping in pyproject.toml to support the new functionality.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| wiki/Realisations.md | Adds CLI usage and help text for the new rupture path plotting tool. |
| visualisation/realisation.py | Updates plot_sources to allow additional keyword arguments. |
| visualisation/plot_rupture_path.py | Implements rupture path plotting and file output via a new CLI command. |
| tests/test_plots.py | Adds a new test for the rupture path plotting tool. |
| pyproject.toml | Registers the new CLI command for plotting rupture paths. |
Comments suppressed due to low confidence (1)
tests/test_plots.py:362
- [nitpick] The variable 'velocity_model_plot_file' is used as the realisation file for the rupture path plot, which may be misleading. Consider renaming it to 'realisation_ffp' for clarity.
plot_rupture_path.plot_rupture_path_to_file(velocity_model_plot_file, output_image_path)
AndrewRidden-Harper
left a comment
There was a problem hiding this comment.
small doc string suggestiong but I'm happy to approve regardless
Co-authored-by: AndrewRidden-Harper <52001209+AndrewRidden-Harper@users.noreply.github.com>
This PR adds a tool to plot realisation rupture paths for realisations without SRFs. This is useful mostly for debugging and exploratory purposes.
See the wiki for usage and documentation details. This PR depends on a couple of others for context so I will wait for those to merge first.