Uh oh!
There was an error while loading. Please reload this page.
GEOPY-2731: Replace InputFile for BaseUIJson class - #56
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates surface-app drivers to use geoapps_utils.base.Driver directly (removing the custom BaseSurfaceDriver/InputFile pathway) as part of the UIJson-related refactor, and refreshes dependency pins/locks accordingly.
Changes:
- Switch
iso_surfacesandsurface_normalsdrivers to inherit fromgeoapps_utils.base.Driverand align behavior with the new base driver pattern (e.g., returning created objects). - Remove
surface_apps/driver.py(BaseSurfaceDriver) and associatedInputFilehandling. - Update
geoh5py/geoapps-utilsgit refs inpyproject.tomland regenerate conda-lock files.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| surface_apps/surface_normals/driver.py | Migrates to geoapps_utils.base.Driver and returns generated Points outputs. |
| surface_apps/iso_surfaces/driver.py | Migrates to geoapps_utils.base.Driver and refactors the main execution method (now run). |
| surface_apps/driver.py | Removes the custom BaseSurfaceDriver abstraction and InputFile-based parameter building. |
| pyproject.toml | Updates geoh5py and geoapps-utils git refs to the UIJson feature branch. |
| environments/py-3.13-win-64.conda.lock.yml | Regenerated lockfile reflecting updated dependency resolution. |
| environments/py-3.13-win-64-dev.conda.lock.yml | Regenerated dev lockfile reflecting updated dependency resolution. |
| environments/py-3.13-linux-64.conda.lock.yml | Regenerated lockfile reflecting updated dependency resolution. |
| environments/py-3.13-linux-64-dev.conda.lock.yml | Regenerated dev lockfile reflecting updated dependency resolution. |
| environments/py-3.12-win-64.conda.lock.yml | Regenerated lockfile reflecting updated dependency resolution. |
| environments/py-3.12-win-64-dev.conda.lock.yml | Regenerated dev lockfile reflecting updated dependency resolution. |
| environments/py-3.12-linux-64.conda.lock.yml | Regenerated lockfile reflecting updated dependency resolution. |
| environments/py-3.12-linux-64-dev.conda.lock.yml | Regenerated dev lockfile reflecting updated dependency resolution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## develop #56 +/- ##
===========================================
+ Coverage 77.40% 84.95% +7.55%
===========================================
Files 5 5 Lines 239 246 +7 Branches 29 30 +1 ===========================================
+ Hits 185 209 +24 + Misses 44 22 -22 - Partials 10 15 +5
🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
gmcga
left a comment
There was a problem hiding this comment.
See my comment in iso_surfaces.driver
I checked the test coverage and noticed the branch coverage in particular seems pretty low for the iso_surfaces driver and options, so I would think it probably makes sense to bolster the unit tests before proceeding/merging.
I didn't think to look at this before but the docs are also outdated due to some things being removed - there are still references to surface_apps/driver.py which is referenced in api_reference.rst and usage.rst, and surface_apps.iso_surfaces.params referenced in iso_surfaces.rst
Uh oh!
There was an error while loading. Please reload this page.
gmcga
left a comment
There was a problem hiding this comment.
Everything looks good, the only issue now is the ferreus files that seem to have slipped in here
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
GEOPY-2731 - Replace InputFile for BaseUIJson class