Skip to content

Conversation

domfournier
Copy link
Collaborator

@domfournier domfournier commented May 15, 2025

GEOPY-2143 - ensure backward compatibility of saved ui.json from Analyst 4.5 to 4.6

@domfournier domfournier requested a review from sebhmg May 15, 2025 19:50
@github-actions github-actions bot changed the title GEOPY-2143 GEOPY-2143: ensure backward compatibility of saved ui.json from Analyst 4.5 to 4.6 May 15, 2025
@domfournier domfournier changed the base branch from develop to release/0.3.0 May 15, 2025 19:50
Copy link
Contributor

@sebhmg sebhmg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to have this!

But... what is the fix which make them backward compatible? I thought there was a problem to address beyond having tests.

Also, the legacy ui.json files must be moved under the test folder.
Please see a couple of other questions.

@@ -50,7 +50,7 @@ def test_airborne_tem_1d_fwr_run(
n_lines=n_grid_points,
cell_size=cell_size,
refinement=refinement,
inversion_type="airborne_tem 1d",
inversion_type="airborne tdem 1d",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: is it changing back to the legacy inversion type?

or it this one the new one, which was already backward compatibly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this helper function to use the same strings as stored on geoh5, so that the test for the legacy uijson can build the demos directly. These were hardcoded before.

The airborne tdem 1d is new to 0.3.0, so not part of legacy.

Copy link

codecov bot commented May 16, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes missing coverage. Please review.

Project coverage is 90.46%. Comparing base (7baad45) to head (62eaa23).
Report is 15 commits behind head on release/0.3.0.

Files with missing lines Patch % Lines
simpeg_drivers/driver.py 66.66% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           release/0.3.0     #211      +/-   ##
=================================================
+ Coverage          90.36%   90.46%   +0.09%     
=================================================
  Files                 86       85       -1     
  Lines               5107     4927     -180     
  Branches             657      636      -21     
=================================================
- Hits                4615     4457     -158     
+ Misses               329      308      -21     
+ Partials             163      162       -1     
Files with missing lines Coverage Δ
simpeg_drivers/driver.py 85.46% <66.66%> (-1.15%) ⬇️

... 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.

@domfournier
Copy link
Collaborator Author

domfournier commented May 16, 2025

But... what is the fix which make them backward compatible? I thought there was a problem to address beyond having tests.

This test confirms that all legacy (0.2.0) jsons are already compatible with 0.3.0 - at least running defaults with core parameters.

We will continue testing going forward to make sure that future changes will preserve the compatibility.

@domfournier domfournier merged commit b95dab0 into release/0.3.0 May 20, 2025
14 checks passed
@domfournier domfournier deleted the GEOPY-2143 branch May 20, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants