Conversation
There was a problem hiding this comment.
Pull request overview
Adds downhole plotting utilities to the drillhole subsystem, enabling quick visualization of interval/point table variables per hole (line, categorical, and numeric “image” styles), plus an example script demonstrating usage.
Changes:
- Added
plot_downholetoDrillholeDatabasefor multi-hole plotting with grid/column layouts and shared categorical legends. - Added downhole sampling helpers and
plot_downholetoDrillHolefor per-hole plotting. - Added
examples/plot_downhole.pydemonstrating line, categorical, and image plotting workflows.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
loopresources/drillhole/drillhole_database.py |
Adds multi-hole downhole plotting + shared categorical legend helper. |
loopresources/drillhole/drillhole.py |
Adds per-hole downhole sampling utilities and plotting API. |
examples/plot_downhole.py |
New example showcasing downhole plot variants and legend usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if show_legend and legend_loc != "none": | ||
| handles = [ | ||
| mpatches.Patch(color=cmap_obj(i), label=str(cat)) | ||
| for i, cat in enumerate(categories) | ||
| ] |
There was a problem hiding this comment.
legend_loc is documented as {"right", "bottom", "none"} but it isn’t normalized/validated, so values like "Right" or typos silently skip adding a legend. Consider legend_loc = legend_loc.lower() and raising a ValueError when it’s not one of the supported values.
| def plot_downhole( | ||
| self, | ||
| table_name: str, | ||
| column: str, | ||
| holes: Optional[List[str]] = None, |
There was a problem hiding this comment.
New plotting behavior is introduced here, but there are no tests covering the expected outputs (e.g., categorical vs numeric sampling, missing column/table errors, and multi-hole layout handling). Since this repo already has substantial tests/test_drillhole_database*.py coverage, adding a few targeted unit tests that validate the sampled depth/value arrays (without asserting on rendered Matplotlib output) would help prevent regressions.
| def _sample_downhole_values( | ||
| self, | ||
| table_name: str, | ||
| column: str, | ||
| step: float, |
There was a problem hiding this comment.
These new sampling helpers are core logic that the plotting APIs depend on, but there are no unit tests exercising their outputs for interval vs point tables (and categorical vs numeric data). Consider adding tests that assert the generated depth grid and sampled values (plus error cases like unknown tables / missing columns) to avoid relying on Matplotlib rendering in tests.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.