Skip to content

Commit e202028

Browse files
committed
Slight rework on uploading. Fixes: multiple measurement or other files in PEtab problem.
Previously: Silently ignored the other files
1 parent 8cc6bfd commit e202028

File tree

3 files changed

+966
-31
lines changed

3 files changed

+966
-31
lines changed

src/petab_gui/controllers/mother_controller.py

Lines changed: 228 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,109 @@ def _open_file(self, actionable, file_path, sep, mode):
732732
file_path, mode, sep
733733
)
734734

735+
def _validate_yaml_structure(self, yaml_content):
736+
"""Validate PEtab YAML structure before attempting to load files.
737+
738+
Parameters
739+
----------
740+
yaml_content : dict
741+
The parsed YAML content.
742+
743+
Returns
744+
-------
745+
tuple
746+
(is_valid: bool, errors: list[str])
747+
"""
748+
errors = []
749+
750+
# Check format version
751+
if "format_version" not in yaml_content:
752+
errors.append("Missing 'format_version' field")
753+
754+
# Check problems array
755+
if "problems" not in yaml_content:
756+
errors.append("Missing 'problems' field")
757+
return False, errors
758+
759+
if (
760+
not isinstance(yaml_content["problems"], list)
761+
or not yaml_content["problems"]
762+
):
763+
errors.append("'problems' must be a non-empty list")
764+
return False, errors
765+
766+
problem = yaml_content["problems"][0]
767+
768+
# Required fields
769+
if "sbml_files" not in problem or not problem["sbml_files"]:
770+
errors.append("Problem must contain at least one SBML file")
771+
772+
# Optional but recommended fields
773+
for field in [
774+
"measurement_files",
775+
"observable_files",
776+
"condition_files",
777+
]:
778+
if field not in problem or not problem[field]:
779+
errors.append(f"Warning: No {field} specified")
780+
781+
# Check parameter_file (at root level)
782+
if "parameter_file" not in yaml_content:
783+
errors.append("Missing 'parameter_file' at root level")
784+
785+
return len([e for e in errors if "Warning" not in e]) == 0, errors
786+
787+
def _validate_files_exist(self, yaml_dir, yaml_content):
788+
"""Validate that all files referenced in YAML exist.
789+
790+
Parameters
791+
----------
792+
yaml_dir : Path
793+
The directory containing the YAML file.
794+
yaml_content : dict
795+
The parsed YAML content.
796+
797+
Returns
798+
-------
799+
tuple
800+
(all_exist: bool, missing_files: list[str])
801+
"""
802+
missing_files = []
803+
problem = yaml_content["problems"][0]
804+
805+
# Check SBML files
806+
for sbml_file in problem.get("sbml_files", []):
807+
if not (yaml_dir / sbml_file).exists():
808+
missing_files.append(str(sbml_file))
809+
810+
# Check measurement files
811+
for meas_file in problem.get("measurement_files", []):
812+
if not (yaml_dir / meas_file).exists():
813+
missing_files.append(str(meas_file))
814+
815+
# Check observable files
816+
for obs_file in problem.get("observable_files", []):
817+
if not (yaml_dir / obs_file).exists():
818+
missing_files.append(str(obs_file))
819+
820+
# Check condition files
821+
for cond_file in problem.get("condition_files", []):
822+
if not (yaml_dir / cond_file).exists():
823+
missing_files.append(str(cond_file))
824+
825+
# Check parameter file
826+
if "parameter_file" in yaml_content:
827+
param_file = yaml_content["parameter_file"]
828+
if not (yaml_dir / param_file).exists():
829+
missing_files.append(str(param_file))
830+
831+
# Check visualization files (optional)
832+
for vis_file in problem.get("visualization_files", []):
833+
if not (yaml_dir / vis_file).exists():
834+
missing_files.append(str(vis_file))
835+
836+
return len(missing_files) == 0, missing_files
837+
735838
def open_yaml_and_load_files(self, yaml_path=None, mode="overwrite"):
736839
"""Open files from a YAML configuration.
737840
@@ -749,62 +852,161 @@ def open_yaml_and_load_files(self, yaml_path=None, mode="overwrite"):
749852
if controller == self.sbml_controller:
750853
continue
751854
controller.release_completers()
855+
752856
# Load the YAML content
753-
with open(yaml_path) as file:
857+
with open(yaml_path, encoding="utf-8") as file:
754858
yaml_content = yaml.safe_load(file)
755859

860+
# Validate PEtab version
756861
if (major := get_major_version(yaml_content)) != 1:
757862
raise ValueError(
758863
f"Only PEtab v1 problems are currently supported. "
759864
f"Detected version: {major}.x."
760865
)
761866

867+
# Validate YAML structure
868+
is_valid, errors = self._validate_yaml_structure(yaml_content)
869+
if not is_valid:
870+
error_msg = "Invalid YAML structure:\n - " + "\n - ".join(
871+
[e for e in errors if "Warning" not in e]
872+
)
873+
self.logger.log_message(error_msg, color="red")
874+
QMessageBox.critical(
875+
self.view, "Invalid PEtab YAML", error_msg
876+
)
877+
return
878+
879+
# Log warnings but continue
880+
warnings = [e for e in errors if "Warning" in e]
881+
for warning in warnings:
882+
self.logger.log_message(warning, color="orange")
883+
762884
# Resolve the directory of the YAML file to handle relative paths
763885
yaml_dir = Path(yaml_path).parent
764886

765-
# Upload SBML model
766-
sbml_file_path = (
767-
yaml_dir / yaml_content["problems"][0]["sbml_files"][0]
768-
)
769-
self.sbml_controller.overwrite_sbml(sbml_file_path)
770-
self.measurement_controller.open_table(
771-
yaml_dir / yaml_content["problems"][0]["measurement_files"][0]
772-
)
773-
self.observable_controller.open_table(
774-
yaml_dir / yaml_content["problems"][0]["observable_files"][0]
887+
# Validate file existence
888+
all_exist, missing_files = self._validate_files_exist(
889+
yaml_dir, yaml_content
775890
)
776-
self.parameter_controller.open_table(
777-
yaml_dir / yaml_content["parameter_file"]
778-
)
779-
self.condition_controller.open_table(
780-
yaml_dir / yaml_content["problems"][0]["condition_files"][0]
781-
)
782-
# Visualization is optional
783-
vis_path = yaml_content["problems"][0].get("visualization_files")
784-
if vis_path:
785-
self.visualization_controller.open_table(
786-
yaml_dir / vis_path[0]
891+
if not all_exist:
892+
error_msg = (
893+
"The following files referenced in the YAML are missing:\n - "
894+
+ "\n - ".join(missing_files)
787895
)
896+
self.logger.log_message(error_msg, color="red")
897+
QMessageBox.critical(self.view, "Missing Files", error_msg)
898+
return
899+
900+
problem = yaml_content["problems"][0]
901+
902+
# Load SBML model (required, single file)
903+
sbml_files = problem.get("sbml_files", [])
904+
if sbml_files:
905+
sbml_file_path = yaml_dir / sbml_files[0]
906+
self.sbml_controller.overwrite_sbml(sbml_file_path)
907+
self.logger.log_message(
908+
f"Loaded SBML file: {sbml_files[0]}", color="blue"
909+
)
910+
911+
# Load measurement files (multiple allowed)
912+
measurement_files = problem.get("measurement_files", [])
913+
if measurement_files:
914+
for i, meas_file in enumerate(measurement_files):
915+
file_mode = "overwrite" if i == 0 else "append"
916+
self.measurement_controller.open_table(
917+
yaml_dir / meas_file, mode=file_mode
918+
)
919+
self.logger.log_message(
920+
f"Loaded measurement file ({i + 1}/{len(measurement_files)}): {meas_file}",
921+
color="blue",
922+
)
923+
924+
# Load observable files (multiple allowed)
925+
observable_files = problem.get("observable_files", [])
926+
if observable_files:
927+
for i, obs_file in enumerate(observable_files):
928+
file_mode = "overwrite" if i == 0 else "append"
929+
self.observable_controller.open_table(
930+
yaml_dir / obs_file, mode=file_mode
931+
)
932+
self.logger.log_message(
933+
f"Loaded observable file ({i + 1}/{len(observable_files)}): {obs_file}",
934+
color="blue",
935+
)
936+
937+
# Load condition files (multiple allowed)
938+
condition_files = problem.get("condition_files", [])
939+
if condition_files:
940+
for i, cond_file in enumerate(condition_files):
941+
file_mode = "overwrite" if i == 0 else "append"
942+
self.condition_controller.open_table(
943+
yaml_dir / cond_file, mode=file_mode
944+
)
945+
self.logger.log_message(
946+
f"Loaded condition file ({i + 1}/{len(condition_files)}): {cond_file}",
947+
color="blue",
948+
)
949+
950+
# Load parameter file (required, single file at root level)
951+
if "parameter_file" in yaml_content:
952+
param_file = yaml_content["parameter_file"]
953+
self.parameter_controller.open_table(yaml_dir / param_file)
954+
self.logger.log_message(
955+
f"Loaded parameter file: {param_file}", color="blue"
956+
)
957+
958+
# Load visualization files (optional, multiple allowed)
959+
visualization_files = problem.get("visualization_files", [])
960+
if visualization_files:
961+
for i, vis_file in enumerate(visualization_files):
962+
file_mode = "overwrite" if i == 0 else "append"
963+
self.visualization_controller.open_table(
964+
yaml_dir / vis_file, mode=file_mode
965+
)
966+
self.logger.log_message(
967+
f"Loaded visualization file ({i + 1}/{len(visualization_files)}): {vis_file}",
968+
color="blue",
969+
)
788970
else:
789971
self.visualization_controller.clear_table()
972+
790973
# Simulation should be cleared
791974
self.simulation_controller.clear_table()
975+
792976
self.logger.log_message(
793977
"All files opened successfully from the YAML configuration.",
794978
color="green",
795979
)
796980
self.check_model()
797-
# rerun the completers
981+
982+
# Rerun the completers
798983
for controller in self.controllers:
799984
if controller == self.sbml_controller:
800985
continue
801986
controller.setup_completers()
802987
self.unsaved_changes_change(False)
803988

989+
except FileNotFoundError as e:
990+
error_msg = f"File not found: {e.filename if hasattr(e, 'filename') else str(e)}"
991+
self.logger.log_message(error_msg, color="red")
992+
QMessageBox.warning(self.view, "File Not Found", error_msg)
993+
except KeyError as e:
994+
error_msg = f"Missing required field in YAML: {str(e)}"
995+
self.logger.log_message(error_msg, color="red")
996+
QMessageBox.warning(self.view, "Invalid YAML", error_msg)
997+
except ValueError as e:
998+
error_msg = f"Invalid YAML structure: {str(e)}"
999+
self.logger.log_message(error_msg, color="red")
1000+
QMessageBox.warning(self.view, "Invalid YAML", error_msg)
1001+
except yaml.YAMLError as e:
1002+
error_msg = f"YAML parsing error: {str(e)}"
1003+
self.logger.log_message(error_msg, color="red")
1004+
QMessageBox.warning(self.view, "YAML Parsing Error", error_msg)
8041005
except Exception as e:
805-
self.logger.log_message(
806-
f"Failed to open files from YAML: {str(e)}", color="red"
807-
)
1006+
error_msg = f"Unexpected error loading YAML: {str(e)}"
1007+
self.logger.log_message(error_msg, color="red")
1008+
logging.exception("Full traceback for YAML loading error:")
1009+
QMessageBox.critical(self.view, "Error", error_msg)
8081010

8091011
def open_omex_and_load_files(self, omex_path=None):
8101012
"""Opens a petab problem from a COMBINE Archive."""

src/petab_gui/models/pandas_table_model.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ class MeasurementModel(PandasTableModel):
10951095
possibly_new_observable = Signal(str) # Signal for new observable
10961096

10971097
def __init__(self, data_frame, type: str = "measurement", parent=None):
1098-
allowed_columns = COLUMNS[type]
1098+
allowed_columns = COLUMNS[type].copy()
10991099
super().__init__(
11001100
data_frame=data_frame,
11011101
allowed_columns=allowed_columns,
@@ -1128,7 +1128,7 @@ class ObservableModel(IndexedPandasTableModel):
11281128
def __init__(self, data_frame, parent=None):
11291129
super().__init__(
11301130
data_frame=data_frame,
1131-
allowed_columns=COLUMNS["observable"],
1131+
allowed_columns=COLUMNS["observable"].copy(),
11321132
table_type="observable",
11331133
parent=parent,
11341134
)
@@ -1140,7 +1140,7 @@ class ParameterModel(IndexedPandasTableModel):
11401140
def __init__(self, data_frame, parent=None, sbml_model=None):
11411141
super().__init__(
11421142
data_frame=data_frame,
1143-
allowed_columns=COLUMNS["parameter"],
1143+
allowed_columns=COLUMNS["parameter"].copy(),
11441144
table_type="parameter",
11451145
parent=parent,
11461146
)
@@ -1153,9 +1153,11 @@ class ConditionModel(IndexedPandasTableModel):
11531153
"""Table model for the condition data."""
11541154

11551155
def __init__(self, data_frame, parent=None):
1156+
# Use a copy to avoid mutating the global COLUMNS constant
1157+
condition_columns = COLUMNS["condition"].copy()
11561158
super().__init__(
11571159
data_frame=data_frame,
1158-
allowed_columns=COLUMNS["condition"],
1160+
allowed_columns=condition_columns,
11591161
table_type="condition",
11601162
parent=parent,
11611163
)
@@ -1206,7 +1208,7 @@ class VisualizationModel(PandasTableModel):
12061208
def __init__(self, data_frame, parent=None):
12071209
super().__init__(
12081210
data_frame=data_frame,
1209-
allowed_columns=COLUMNS["visualization"],
1211+
allowed_columns=COLUMNS["visualization"].copy(),
12101212
table_type="visualization",
12111213
parent=parent,
12121214
)

0 commit comments

Comments
 (0)