Address comments

Author: frode-aarstad

src/ert/gui/tools/manage_experiments/export_parameters_dialog.py

@@ -74,7 +74,7 @@ def export(self) -> None:
         output_file: str = self._file_path_edit.text().strip()
         try:
             self._export_button.setEnabled(False)
-            parameters_df = self._ensemble.load_all_scalar_keys(transformed=True)
+            parameters_df = self._ensemble.load_scalar_keys(transformed=True)
             parameters_df.write_csv(output_file, float_precision=6)
             self._export_text_area.insertPlainText(
                 f"Ensemble parameters exported to: {output_file}\n"
@@ -85,9 +85,6 @@ def export(self) -> None:
                 f"Error exporting ensemble parameters: {e!s}"
                 "</span><br>"
             )
-            self._export_text_area.insertPlainText(
-                f"Error exporting ensemble parameters: {e!s}\n"
-            )
         finally:
             self._export_button.setEnabled(True)
 
@@ -113,13 +110,12 @@ def _set_valid() -> None:
             self._file_path_edit.setToolTip("")
             self._export_button.setEnabled(True)
 
-        file_path = self._file_path_edit.text().strip()
-        if not file_path or file_path.endswith(("/", "\\")):
+        path = Path(self._file_path_edit.text().strip())
+        if path.is_dir():
             _set_invalid()
             return
 
         try:
-            path = Path(file_path)
             if path.parent.exists() and path.parent.is_dir():
                 _set_valid()
                 return

src/ert/gui/tools/manage_experiments/storage_info_widget.py

@@ -1,3 +1,4 @@
+import contextlib
 import json
 from enum import IntEnum
 
@@ -388,19 +389,25 @@ def _currentTabChanged(self, index: int) -> None:
         elif index == _EnsembleWidgetTabs.PARAMETERS_TAB:
             assert self._ensemble is not None
 
-            parameters_df = self._ensemble.load_all_scalar_keys(transformed=True)
+            df: pl.DataFrame = pl.DataFrame()
+            with contextlib.suppress(Exception):
+                df = self._ensemble.load_scalar_keys(transformed=True)
 
-            self._parameters_table.setRowCount(len(parameters_df))
-            self._parameters_table.setColumnCount(len(parameters_df.columns))
-            self._parameters_table.setHorizontalHeaderLabels(parameters_df.columns)
+            table = self._parameters_table
 
-            for row_idx in range(len(parameters_df)):
-                for col_idx, col_name in enumerate(parameters_df.columns):
-                    value = parameters_df[col_name][row_idx]
-                    table_widget_item = QTableWidgetItem(str(value))
-                    self._parameters_table.setItem(row_idx, col_idx, table_widget_item)
+            table.setUpdatesEnabled(False)
+            table.setSortingEnabled(False)
+            table.setRowCount(df.height)
+            table.setColumnCount(df.width)
+            table.setHorizontalHeaderLabels(df.columns)
 
-            self._parameters_table.resizeColumnsToContents()
+            rows = df.rows()
+            for r, row in enumerate(rows):
+                for c, v in enumerate(row):
+                    table.setItem(r, c, QTableWidgetItem("" if v is None else str(v)))
+
+            table.resizeColumnsToContents()
+            table.setUpdatesEnabled(True)
 
     @Slot(Ensemble)
     def setEnsemble(self, ensemble: Ensemble) -> None:

src/ert/run_models/_create_run_path.py

@@ -126,7 +126,7 @@ def _generate_parameter_files(
     ]
     scalar_data: dict[str, float | str] = {}
     if keys:
-        df = fs._load_scalar_keys(keys=keys, realizations=iens, transformed=True)
+        df = fs.load_scalar_keys(keys=keys, realizations=iens, transformed=True)
         scalar_data = df.to_dicts()[0]
     exports: dict[str, dict[str, float | str]] = {}
     for param in parameter_configs:

src/ert/storage/local_ensemble.py

@@ -562,21 +562,15 @@ def _load_parameters_lazy(
         df = pl.scan_parquet(group_path)
         return df
 
-    def load_all_scalar_keys(self, transformed: bool = False) -> pl.DataFrame:
-        try:
-            return self._load_scalar_keys(
-                keys=self.experiment.parameter_keys,
-                transformed=transformed,
-            )
-        except KeyError:
-            return pl.DataFrame([[np.nan] * len(self.experiment.parameter_keys)])
-
-    def _load_scalar_keys(
+    def load_scalar_keys(
         self,
-        keys: list[str],
+        keys: list[str] | None = None,
         realizations: int | npt.NDArray[np.int_] | None = None,
         transformed: bool = False,
     ) -> pl.DataFrame:
+        if keys is None:
+            keys = self.experiment.parameter_keys
+
         df_lazy = self._load_parameters_lazy(SCALAR_FILENAME)
         df_lazy = df_lazy.select(["realization", *keys])
         if realizations is not None:
@@ -629,7 +623,7 @@ def load_parameters(
         cardinality = next(cfg.cardinality for cfg in cfgs)
 
         if cardinality == ParameterCardinality.multiple_configs_per_ensemble_dataset:
-            return self._load_scalar_keys(
+            return self.load_scalar_keys(
                 [cfg.name for cfg in cfgs], realizations, transformed
             )
         return self._load_dataset(
@@ -656,7 +650,7 @@ def load_parameters_numpy(
         ]
         if keys:
             return (
-                self._load_scalar_keys(keys, realizations)
+                self.load_scalar_keys(keys, realizations)
                 .drop("realization")
                 .to_numpy()
                 .T.copy()

tests/ert/ui_tests/gui/test_export_parameters_dialog.py

@@ -12,7 +12,7 @@
 from ert.storage import Storage
 
 
-def test_that_export_writes_to_file(
+def test_that_export_writes_to_file_and_check_the_file_contents(
     qtbot, snake_oil_storage: Storage, change_to_tmpdir
 ):
     ensemble = next(snake_oil_storage.ensembles)
@@ -34,7 +34,7 @@ def test_that_export_writes_to_file(
     )
 
     assert_frame_equal(
-        ensemble.load_all_scalar_keys(transformed=True),
+        ensemble.load_scalar_keys(transformed=True),
         pl.read_csv("test_export.csv"),
         abs_tol=1e-6,
     )
@@ -46,12 +46,12 @@ def test_that_export_writes_to_file(
         "",
         "/non/existent/path/export.csv",
         "   ",
-        "hhhee/\0invalid.csv",
         "/",
-        "\\",
     ],
 )
-def test_file_path_validation_invalid(qtbot, snake_oil_storage: Storage, invalid_path):
+def test_that_file_path_validation_fails_on_invalid_paths(
+    qtbot, snake_oil_storage: Storage, invalid_path
+):
     ensemble = next(snake_oil_storage.ensembles)
     dialog = ExportParametersDialog(ensemble)
     qtbot.addWidget(dialog)
@@ -72,9 +72,10 @@ def test_file_path_validation_invalid(qtbot, snake_oil_storage: Storage, invalid
         "valid_export.csv",
         "   valid_export.csv   ",
         "subdir/valid_export.csv",
+        "valid-export",
     ],
 )
-def test_file_path_validation_valid(
+def test_that_file_path_validation_succeeds_on_valid_paths(
     qtbot, snake_oil_storage: Storage, valid_path, change_to_tmpdir
 ):
     ensemble = next(snake_oil_storage.ensembles)
@@ -92,11 +93,11 @@ def test_file_path_validation_valid(
     assert not dialog._file_path_edit.toolTip()
 
 
-@patch("ert.storage.Ensemble.load_all_scalar_keys")
-def test_export_failure_handling(
-    patched_load_all_scalar_keys, qtbot, snake_oil_storage: Storage
+@patch("ert.storage.Ensemble.load_scalar_keys")
+def test_that_export_failure_is_handled_correctly(
+    patched_load_scalar_keys, qtbot, snake_oil_storage: Storage
 ):
-    patched_load_all_scalar_keys.side_effect = Exception("i_am_an_exception")
+    patched_load_scalar_keys.side_effect = Exception("i_am_an_exception")
 
     ensemble = next(snake_oil_storage.ensembles)
     dialog = ExportParametersDialog(ensemble)

tests/ert/ui_tests/gui/test_manage_experiments_tool.py

@@ -550,7 +550,7 @@ def test_realization_view(
     } == set(realization_widget._parameter_text_edit.toPlainText().strip().splitlines())
 
 
-def test_parameters_pane(
+def test_that_parameters_pane_is_populated_correctly(
     qtbot, snake_oil_case_storage: ErtConfig, snake_oil_storage: Storage
 ):
     config = snake_oil_case_storage
@@ -589,7 +589,7 @@ def test_parameters_pane(
     )
 
 
-def test_export_parameters_button(
+def test_that_export_parameters_button_opens_the_export_dialog(
     qtbot, snake_oil_case_storage: ErtConfig, snake_oil_storage: Storage
 ):
     config = snake_oil_case_storage