GEOPY-2739: Accept BaseUIJson in the start of driver by domfournier · Pull Request #190 · MiraGeoscience/geoapps-utils

domfournier · 2026-03-31T23:12:37Z

GEOPY-2739 - Accept BaseUIJson in the start of driver

# Conflicts: # geoapps_utils/base.py

geoapps_utils/run.py

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

Copilot

Pull request overview

This PR updates geoapps-utils to work with geoh5py.ui_json.UIJson as the primary ui.json representation (instead of InputFile/BaseUIJson), including allowing drivers to start from a UIJson instance and updating related templates and tests.

Changes:

Switch driver startup and UIJsonGroup execution flow to use UIJson objects directly.
Refactor Options to derive/serialize UIJson via a default template (base.ui.json) and adjust tests accordingly.
Update the base ui.json asset schema for out_group and pin geoh5py to a GEOPY-2739 revision.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`geoapps_utils/base.py`	Refactors `Driver.start` and `Options` to operate around `UIJson`, adds deprecation path for `InputFile`, and updates serialization/out_group saving.
`geoapps_utils/run.py`	Updates `run_uijson_group` and workspace/ui.json copy helpers to use `UIJson.read/from_dict/write`.
`geoapps_utils-assets/uijson/base.ui.json`	Updates the base template to represent `out_group` as a structured group parameter object.
`tests/uijson_run_test.py`	Updates test setup to build and write a `UIJson` file directly and adjusts expected filenames.
`tests/driver_test.py`	Aligns tests with new UIJson-based serialization behavior and updated exception types.
`tests/dataclass_test.py`	Loosens flatten equality assertions to avoid direct `geoh5` comparisons and updates UIJson writing callsites.
`pyproject.toml`	Pins `geoh5py` dependency to `rev = "GEOPY-2739"`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

geoapps_utils/base.py

Copilot · 2026-04-02T21:08:03Z

pyproject.toml

 ## pip dependencies from Git repositories
 #----------------------------------------
 #geoh5py = {version = ">=0.13.0a, 0.13.*", source = "pypi", allow-prereleases = true}
-geoh5py = {git = "https://github.com/MiraGeoscience/geoh5py.git", rev = "develop"}
+geoh5py = {git = "https://github.com/MiraGeoscience/geoh5py.git", rev = "GEOPY-2739"}



Pinning geoh5py to a moving branch name (rev = "GEOPY-2739") can make builds non-reproducible if the branch head changes. Prefer pinning to an immutable commit hash (or a released version/tag) once the needed changes are available.

yep, this should not be merged like this!

geoapps_utils/base.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

for more information, see https://pre-commit.ci

codecov · 2026-04-02T21:17:02Z

Codecov Report

❌ Patch coverage is 70.76923% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.40%. Comparing base (eb04741) to head (11b1282).

Files with missing lines	Patch %	Lines
geoapps_utils/base.py	67.34%	11 Missing and 5 partials ⚠️
geoapps_utils/run.py	81.25%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           develop     #190       +/-   ##
============================================
- Coverage    86.14%   75.40%   -10.75%     
============================================
  Files           19       19               
  Lines         1126     1114       -12     
  Branches       151      151               
============================================
- Hits           970      840      -130     
- Misses         116      235      +119     
+ Partials        40       39        -1

Files with missing lines	Coverage Δ
geoapps_utils/run.py	`90.99% <81.25%> (-1.61%)`	⬇️
geoapps_utils/base.py	`78.31% <67.34%> (-13.40%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…s-utils into GEOPY-2739

MatthieuCMira

I have the feeling the tests were supress without being replaced.

As this is at the root of all our apps, it shoulkd be heavily unitested right?

!!

MatthieuCMira · 2026-04-03T15:31:51Z

pyproject.toml

 ## pip dependencies from Git repositories
 #----------------------------------------
 #geoh5py = {version = ">=0.13.0a, 0.13.*", source = "pypi", allow-prereleases = true}
-geoh5py = {git = "https://github.com/MiraGeoscience/geoh5py.git", rev = "develop"}
+geoh5py = {git = "https://github.com/MiraGeoscience/geoh5py.git", rev = "GEOPY-2739"}



yep, this should not be merged like this!

MatthieuCMira · 2026-04-03T15:34:43Z

tests/driver_test.py

 }


-def test_base_driver(tmp_path):


you cannot just supress those tests without replacing it, can you?
We should test the behaviors with the new UIJson class to check everything is working the same way!

MatthieuCMira

failueres with scigeoh5.
The solution is to open everything in the same workspace.

MatthieuCMira · 2026-04-03T18:13:33Z

geoapps_utils/base.py

+        if ifile.geoh5 is None:
+            raise GeoAppsError("The application needs a valid 'geoh5' file.")
+
+        params = cls._params_class.build(ifile, **kwargs)


This load the objects as geoh5py instances, on my side, it leads to an error.

The error is because i access the valus to do validations in scigeoh5.
But them the workspace is closed, so valeus cannot be loaded

MatthieuCMira · 2026-04-03T18:35:00Z

geoapps_utils/base.py

+            data = input_file_deprecation_warning(input_data)
+
+        if isinstance(data, UIJson):
+            data = data.to_params()


to params is opening its own context.
But it's loading objects, and the driver might want to use them.
We want to pass a workspace in params, so the object are always opens!

MatthieuCMira · 2026-04-03T18:42:20Z

geoapps_utils/base.py

+        if ifile.geoh5 is None:
+            raise GeoAppsError("The application needs a valid 'geoh5' file.")
+
+        params = cls._params_class.build(ifile, **kwargs)


Suggested change

params = cls._params_class.build(ifile, **kwargs)

with Workspace(ifile.geoh5).open(mode=mode) as workspace:

try:

params = cls._params_class.build(ifile, workspace=workspace **kwargs)

open the workspace here,
pass the workspace in build, in build, pass the workspace to "to_params()" method.

So the objects loaded in the options will be the same as the objects in the application.

GEOPY-2739: corrections

domfournier added 4 commits March 23, 2026 10:52

Start deprecating InputFile

5961372

Merge branch 'develop' into GEOPY-2739

694b5b1

# Conflicts: # geoapps_utils/base.py

Start removing InputFile from mechanics

434493a

Re-trigger

d05d6ce

github-actions bot changed the title ~~GEOPY-2739~~ GEOPY-2739: Accept BaseUIJson in the start of driver Mar 31, 2026

Update run functions

bebadf2

github-code-quality bot found potential problems Apr 2, 2026

View reviewed changes

geoapps_utils/run.py Fixed Show fixed Hide fixed

Potential fix for pull request finding 'Unused import'

39f3306

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

domfournier marked this pull request as ready for review April 2, 2026 21:03

Copilot AI review requested due to automatic review settings April 2, 2026 21:03

Copilot started reviewing on behalf of domfournier April 2, 2026 21:04 View session

domfournier added 2 commits April 2, 2026 14:05

Remove test for drepcated Params class

4276096

Merge branch 'GEOPY-2739' of https://github.com/MiraGeoscience/geoapp…

e60ff45

…s-utils into GEOPY-2739

Copilot AI reviewed Apr 2, 2026

View reviewed changes

domfournier and others added 4 commits April 2, 2026 14:08

Remove more ref to TestParams

30ba1c3

Re-lock on geoh5py branch

a5a0192

Apply suggestion from @Copilot

e60c41f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

b11b6ed

for more information, see https://pre-commit.ci

domfournier and others added 4 commits April 2, 2026 14:21

Fixes from copilot

efdf62a

Update geoapps_utils/base.py

b192fab

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Simplify conversion InputFile -> UIjson

bab0d64

Merge branch 'GEOPY-2739' of https://github.com/MiraGeoscience/geoapp…

11b1282

…s-utils into GEOPY-2739

MatthieuCMira requested changes Apr 3, 2026

View reviewed changes

Matthieu Cedou and others added 2 commits April 3, 2026 14:44

do all the operations under the same context

a9e7bec

Merge pull request #191 from MiraGeoscience/GEOPY-2739-Mat

92c740b

GEOPY-2739: corrections

-        params = cls._params_class.build(ifile, **kwargs)
+         with Workspace(ifile.geoh5).open(mode=mode) as workspace:
+                try:
+                        params = cls._params_class.build(ifile, workspace=workspace **kwargs)

		}


		def test_base_driver(tmp_path):

Conversation

domfournier commented Mar 31, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MatthieuCMira left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira left a comment

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

domfournier commented Mar 31, 2026 •

edited by github-actions bot

Loading

codecov bot commented Apr 2, 2026 •

edited

Loading

MatthieuCMira left a comment •

edited

Loading