GEOPY-2746: implement "Group Value" in BaseUIJSON by MatthieuCMira · Pull Request #848 · MiraGeoscience/geoh5py

MatthieuCMira · 2026-03-06T20:30:43Z

GEOPY-2746 - "Group Value" form is not implemented in BaseUIJSON
add a new form for multi Data Group

accept tooltips as list

Copilot

Pull request overview

Adds a new ui.json form type for selecting multiple data items from within a group, and broadens tooltip handling across all forms.

Changes:

Introduces MultiDataGroupForm to represent multi-selection of data UUIDs within a group context.
Adds a BaseForm tooltip validator to accept tooltips provided as a list[str] and normalize them into a single string.
Extends the forms unit tests to cover the new form and tooltip-list behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`geoh5py/ui_json/forms.py`	Adds tooltip normalization and introduces `MultiDataGroupForm` plus updated validator typing/docs.
`tests/ui_json/forms_test.py`	Adds a new unit test for `MultiDataGroupForm` and tooltip list joining.

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

geoh5py/ui_json/forms.py

codecov · 2026-03-06T20:36:02Z

Codecov Report

❌ Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.12%. Comparing base (7e3b1de) to head (1f69968).
⚠️ Report is 16 commits behind head on develop.

Files with missing lines	Patch %	Lines
geoh5py/ui_json/forms.py	92.59%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #848      +/-   ##
===========================================
- Coverage    91.18%   91.12%   -0.07%     
===========================================
  Files          115      115              
  Lines        10272    10292      +20     
  Branches      1898     1900       +2     
===========================================
+ Hits          9367     9379      +12     
- Misses         483      489       +6     
- Partials       422      424       +2

Files with missing lines	Coverage Δ
geoh5py/ui_json/constants.py	`100.00% <ø> (ø)`
geoh5py/ui_json/forms.py	`95.18% <92.59%> (-1.13%)`	⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

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

test it correct a small error in the tests of geo_image_test

Copilot

Pull request overview

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

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

geoh5py/ui_json/forms.py

tests/geo_image_test.py

geoh5py/ui_json/forms.py

tests/ui_json/forms_test.py

geoh5py/ui_json/forms.py

Copilot

Pull request overview

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

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

geoh5py/ui_json/forms.py

benk-mira

You'll have to show me how the extra 'mandatory' counting is necessary. I tested the old infer with the new form type (see comment), and it seems to work without augmenting the infer. Can you add a unit test to my 'test_2746' branch that fails?

geoh5py/ui_json/forms.py

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

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

geoh5py/ui_json/forms.py

Copilot · 2026-03-11T13:04:57Z

geoh5py/ui_json/forms.py

+    :param data: The form data to check for matching indicators.
+
+    :return: An array of candidate subclasses with the most matching indicators.
    """
    counts = count_indicators(INDICATORS, data)


BaseForm.infer() relies on key normalization via to_snake(), but ui.json templates in this repo emit multiselect (all lowercase) rather than multiSelect / multi_select (see geoh5py/ui_json/templates.py). Because to_snake('multiselect') stays multiselect, required indicators like multi_select won’t match, and forms like MultiSelectDataForm can be mis-inferred as DataForm. Consider explicitly normalizing multiselect -> multi_select during inference (or supporting both aliases) and add a regression test.

This templates could be supress no @domfournier ?

…of forms.py

geoh5py/ui_json/forms.py

GEOPY-2746-unit-test-fill

benk-mira

Some suggestions to account for the fact that the form is only used with DrillholeGroup/PropertyGroup

geoh5py/ui_json/forms.py

…into GEOPY-2746

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

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

You can also share your feedback on Copilot code review. Take the survey.

geoh5py/ui_json/constants.py

geoh5py/ui_json/forms.py

Matthieu Cedou added 2 commits March 6, 2026 10:12

add a new form for multi Data Group

d63665c

accept tooltips as list

accept str also in MultiDataGroup

7d44cd7

MatthieuCMira requested review from benk-mira and Copilot March 6, 2026 20:30

Copilot started reviewing on behalf of MatthieuCMira March 6, 2026 20:31 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

geoh5py/ui_json/forms.py Show resolved Hide resolved

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

Matthieu Cedou added 2 commits March 9, 2026 11:39

udpate the form detection to avoid bugs

cf7488c

test it correct a small error in the tests of geo_image_test

small correction to make it works

fc8f232

Copilot AI review requested due to automatic review settings March 9, 2026 17:29

Copilot started reviewing on behalf of MatthieuCMira March 9, 2026 17:29 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

Matthieu Cedou added 3 commits March 9, 2026 13:45

few corrections

0a6e69d

remove the match to geoimage error

573a573

test correciton

f6aec18

Copilot AI review requested due to automatic review settings March 9, 2026 17:57

Copilot started reviewing on behalf of MatthieuCMira March 9, 2026 17:58 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

geoh5py/ui_json/forms.py Show resolved Hide resolved

benk-mira requested changes Mar 10, 2026

View reviewed changes

update based on Ben's comment

d78f6ba

MatthieuCMira requested a review from benk-mira March 10, 2026 21:43

add an ex failure example

48cdfb7

Copilot AI review requested due to automatic review settings March 11, 2026 13:00

Copilot started reviewing on behalf of MatthieuCMira March 11, 2026 13:01 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

Fill infer unit test with all form types and re-order to match order …

bd310a7

…of forms.py

domfournier reviewed Mar 11, 2026

View reviewed changes

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

geoh5py/ui_json/forms.py Outdated Show resolved Hide resolved

benk-mira and others added 2 commits March 11, 2026 11:40

fix type

515b293

Merge pull request #855 from MiraGeoscience/GEOPY-2746-unit-test-fill

beed4c6

GEOPY-2746-unit-test-fill

benk-mira requested changes Mar 11, 2026

View reviewed changes

Matthieu Cedou added 2 commits March 11, 2026 16:52

small last corrections

a11a341

Merge branch 'GEOPY-2746' of https://github.com/MiraGeoscience/geoh5py …

369fd6a

…into GEOPY-2746

Copilot AI review requested due to automatic review settings March 11, 2026 20:54

Copilot started reviewing on behalf of MatthieuCMira March 11, 2026 20:54 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

geoh5py/ui_json/constants.py Outdated Show resolved Hide resolved

geoh5py/ui_json/forms.py Show resolved Hide resolved

geoh5py/ui_json/forms.py Show resolved Hide resolved

copilote's suggestion

1f69968

sebhmg changed the title ~~GEOPY-2746~~ GEOPY-2746: "Group Value" form is not implemented in BaseUIJSON Mar 16, 2026

sebhmg changed the title ~~GEOPY-2746: "Group Value" form is not implemented in BaseUIJSON~~ GEOPY-2746: implement "Group Value" in BaseUIJSON Mar 16, 2026

domfournier merged commit 771ae87 into develop Mar 17, 2026
17 checks passed

domfournier deleted the GEOPY-2746 branch March 17, 2026 18:59

Conversation

MatthieuCMira commented Mar 6, 2026 • edited by sebhmg Loading Uh oh! There was an error while loading. Please reload this page.

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!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benk-mira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

MatthieuCMira Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benk-mira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MatthieuCMira commented Mar 6, 2026 •

edited by sebhmg

Loading

codecov bot commented Mar 6, 2026 •

edited

Loading