Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tree view for 'case view' #7107

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

frode-aarstad
Copy link
Contributor

Issue
Resolves #6722

Approach
Short description of the approach

(Screenshot of new behavior in GUI if applicable)

  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure tests pass locally (after every commit!)

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Create Backport PR to latest release

@frode-aarstad frode-aarstad self-assigned this Feb 5, 2024
@frode-aarstad frode-aarstad force-pushed the storage-tree-view branch 4 times, most recently from 6afe3b0 to ef53920 Compare February 13, 2024 09:43
@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2024

Codecov Report

Attention: Patch coverage is 90.81633% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 85.13%. Comparing base (ce1cc0b) to head (f76ac43).
Report is 5 commits behind head on main.

Files Patch % Lines
src/ert/gui/ertwidgets/models/storage_model.py 80.21% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7107      +/-   ##
==========================================
+ Coverage   84.87%   85.13%   +0.26%     
==========================================
  Files         384      386       +2     
  Lines       22805    22932     +127     
  Branches      945      940       -5     
==========================================
+ Hits        19355    19524     +169     
+ Misses       3332     3294      -38     
+ Partials      118      114       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@frode-aarstad frode-aarstad force-pushed the storage-tree-view branch 2 times, most recently from cbb27b6 to db0a869 Compare February 14, 2024 11:12
@dafeda
Copy link
Contributor

dafeda commented Feb 16, 2024

This is looking good 👍

When creating a new experiment in the "Manage cases" section by hitting the plus-button, users are prompted to input an "ensemble name".
Ert defaults to using the date as the experiment name, so we end up with something like this:

image

Is it difficult to allow users to input an experiment name as well?

@frode-aarstad frode-aarstad force-pushed the storage-tree-view branch 3 times, most recently from 608d4f5 to c349413 Compare February 20, 2024 06:11
@frode-aarstad
Copy link
Contributor Author

Have added a "Create new experiment" dialog. Simple for now

bilde

Copy link
Contributor

@dafeda dafeda left a comment

Choose a reason for hiding this comment

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

I think this gives a nice and useful overview of the experiments and ensembles in storage.
Good stuff 👍
@oyvindeide should probably have a look as well.

Copy link
Collaborator

@oyvindeide oyvindeide left a comment

Choose a reason for hiding this comment

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

Looks like a good change! I have some questions. Probably not related, but if you press initialize from scratch we can overwrite existing parameters? Probably outside the scope of this PR, so I will create a separate issue.

)

(path / "index.json").write_text(_Index(id=uuid, name=name).model_dump_json())
with open(path / cls._simulation_arguments_file, "w", encoding="utf-8") as f:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this require migration?

dialog = wait_for_child(gui, qtbot, ValidatedDialog)
dialog.param_name.setText(case_name)
qtbot.mouseClick(dialog.ok_button, Qt.LeftButton)
# dialog = wait_for_child(gui, qtbot, CreateExperimentDialog, options=Qt.FindChildOption.FindChildrenRecursively)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Stray comment?

@@ -15,6 +15,7 @@ class SimulationArguments:
class SingleTestRunArguments(SimulationArguments):
current_case: str
target_case: Optional[str] = None
analysis_module: str = "Single test"
Copy link
Collaborator

Choose a reason for hiding this comment

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

analysis_module is a bit overloaded, so suggest we rename this ensemble_type instead

@@ -42,6 +44,7 @@ def __post_init__(self) -> None:
class EvaluateEnsembleRunArguments(SimulationArguments):
active_realizations: List[bool]
current_case: str
analysis_module: str = "Evaluate ensemble"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are just depending on this being in simulation arguments, right? So if we want to refactor this we just need to keep this? The reason I am asking is because we might want to remove current_case for example, and would like that to be easy to do.

@frode-aarstad frode-aarstad merged commit 4b81076 into equinor:main Feb 29, 2024
54 checks passed
@frode-aarstad frode-aarstad deleted the storage-tree-view branch February 29, 2024 11:07
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.

Create a MVP for treeview in Manage Cases
4 participants