diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c9c9e7..02b6f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Strings are converted only when the destination column needs it. DateTime columns parse them; Double columns accept a single `.` or `,` as the decimal separator, so `"1,234"` is 1.234, and anything else raises `ValueError` naming the field. An empty string becomes `None` for both. Strings bound for other columns are passed through unchanged. - Minimum versions now follow [SPEC 0](https://scientific-python.org/specs/spec-0000/): numpy 2.3, pandas 2.3 and pythonnet 3.0.5. - Importing `mikeplus` with anything other than x64 Python on Windows or Linux now raises `ImportError`. +- `Database.run()` raises `RuntimeError` when the engine exits with an error or doesn't write its result files, naming the engine's logs (#143). +- `TopoRepairTool.run()` raises `ValueError` unless the active model is CS_MIKE1D or WD_EPANET, instead of doing nothing (#146). +- `CathSlopeLengthProcess.run()` reports the tool's warnings with `warnings.warn` (#145). +- `ScenarioCollection.find_by_name()` returns every scenario with the name, not at most one (#146). +- `Database.close()` returns `None` whether or not the database was open (#146). - The documentation site moved to great-docs. API reference pages moved from `api/` to `reference/`, and the auto-generated table classes no longer have their own pages. User guide URLs are unchanged. ### Removed @@ -42,6 +47,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Inserting a row with no field values on MIKE+ 2026 Update 1. - `InterpolationTool.interpolate_from_neighobour` always raised `AttributeError`; it now honours `alongPath`. - The `InterpolationTool` methods raised `TypeError` when `value_as_missing` was a number. +- `Database.create(overwrite=True)` deleted the existing database before rejecting `srid` together with `projection_string` (#140). +- `to_sql()` and `by_muid()` didn't escape single quotes, and `to_sql(True)` gave `True` rather than `1` (#144). +- Progress from `CathSlopeLengthProcess` and `ConnectionRepairTool` was never printed, because the handler was attached after the tool ran (#145). +- `Alternative.scenarios` raised `AttributeError` when the model had a scenario other than Base, and it, like `Alternative ==`, confused alternatives of different groups that share an id (#142). +- Iterating `db.scenarios` returned scenarios with the same name as one scenario repeated, and iterating an `AlternativeGroup` skipped alternatives below the base's children (#146). - User guide examples and docstrings that raised or contradicted the code, notably `db.run(sim_option=...)` (not `model_option`), `insert()` running immediately, single-use queries, and `db.alternative_groups` (not `db.scenarios.alternative_groups`). ## [2026.0.0] - 2026-01-29 diff --git a/docs/dictionary.txt b/docs/dictionary.txt index f24cb6a..6f50b09 100644 --- a/docs/dictionary.txt +++ b/docs/dictionary.txt @@ -1,9 +1,11 @@ # Words great-docs proofread should accept: domain terms, and identifiers that # appear in prose and docstrings. One per line; matching is case-sensitive. ActiveProduct +Alloc alongPath AlternativeGroup AlternativeGroupCollection +AltId AssignOption BaseNodeTable BaseQuery @@ -18,18 +20,27 @@ CatchSlopeLengthProcess CathSlopeLengthProcess chainage ClosestNode +Coeff1 +Coeff4 ColumnName configFile conn ConnectionRepairTool +ConnectionTypeNo +CreateValvesFromPointsTool +DatabaseError DataFrame DataFrames DataTableContainer DataTables +DBFilePath DbType DeleteQuery DEM dem +DemAlloc +DemAllocConn +DemandAggregationTool DemandConnectionTool DEMs demUnitKey @@ -48,12 +59,17 @@ DTZ001 EngineTool EPANET epanet +ETRS89 f F401 +FCV FileExistsError GA +GenerateTypeNo GetMuidAndFieldsWhereOrder +GetScenarios getsourcefile +GPV GroundLevel IAlternative IAlternativeGroup @@ -71,10 +87,15 @@ InstallRoot InterpolationTool IScenario IScenarioManager +ItemsView JobList +JunctionID +KeysView LD len +LoadPointConnection MaxValueNeighbours +MDemand MIKE1D MikeImport mikeio @@ -100,6 +121,12 @@ noqa NullReference parameterless pathlib +PathLike +PBV +PFS +PipeID +PRV +PSV Pythonnet pythonnet QueryResultT @@ -114,10 +141,12 @@ SimulationRunner sqlite SRID srid +StationCon SWMM swmm TableCollection tabset +TCV testdata tjunction TODO @@ -130,9 +159,12 @@ UpdateQuery UpstreamElement UpstreamMaxValue UpstreamMinValue +UTM uv +ValuesView VC venv +VTYPE WD WGS84 WindowsDesktop diff --git a/docs/user_guide/02-database.qmd b/docs/user_guide/02-database.qmd index d834064..c43d52b 100644 --- a/docs/user_guide/02-database.qmd +++ b/docs/user_guide/02-database.qmd @@ -87,7 +87,7 @@ with mp.open("model.sqlite") as db: It returns the existing project file if there is one and never changes it. A new one records the database's model type and unit system, and refers to the database by a path relative to itself, so keep the two files together when you move the model. ::: {.callout-warning} -`mp.create(..., overwrite=True)` deletes an existing `.mupp` as well as the database. Settings MIKE+ keeps in a `.mupp`, such as map layers, symbology and the map's coordinate system, are lost. +`mp.create(..., overwrite=True)` deletes the `.mupp` of that name as well as the database. Settings MIKE+ keeps in a `.mupp`, such as map layers, symbology and the map's coordinate system, are lost. ::: ### General Database Properties diff --git a/docs/user_guide/06-simulations.qmd b/docs/user_guide/06-simulations.qmd index c61e611..97ea721 100644 --- a/docs/user_guide/06-simulations.qmd +++ b/docs/user_guide/06-simulations.qmd @@ -51,15 +51,13 @@ for path in result_paths: ## {{< icon triangle-alert >}} Error Handling -`db.run()` raises `RuntimeError` if the engine fails to start, and `ValueError` if `sim_option` is invalid or cannot be derived from the active model. - -Errors that occur while the engine is running are not raised. Check the simulation log, or that the result files exist, before using the results. +`db.run()` raises `RuntimeError` if the engine fails to start, exits with an error, or doesn't write its result files; the message names the engine's logs. It raises `ValueError` if `sim_option` is invalid or cannot be derived from the active model. ```python try: result_files = db.run() except RuntimeError as e: - print(f"Simulation failed to start: {e}") + print(f"Simulation failed: {e}") ``` ## {{< icon file-code >}} Example @@ -75,7 +73,7 @@ with mp.open("path/to/your_model.sqlite") as db: try: results = db.run() except RuntimeError as e: - print(f"Simulation failed to start: {e}") + print(f"Simulation failed: {e}") else: missing = [path for path in results if not path.exists()] if missing: diff --git a/mikeplus/database.py b/mikeplus/database.py index 3338f92..ac9cb5f 100644 --- a/mikeplus/database.py +++ b/mikeplus/database.py @@ -15,7 +15,6 @@ from .scenarios.scenario import Scenario from pathlib import Path -from plistlib import InvalidFileException from DHI.Amelia.DataModule.Services.DataSource import BaseDataSource from DHI.Amelia.DataModule.Services.DataSource.ScenarioMangement import ( @@ -59,8 +58,6 @@ def __init__(self, model_path: str | Path, *, auto_open: bool = True): ------ FileNotFoundError If the database file doesn't exist - InvalidFileException - If the model file is invalid """ model_path = Path(model_path) @@ -79,11 +76,8 @@ def __init__(self, model_path: str | Path, *, auto_open: bool = True): if db_file.exists(): self._db_path = db_file - if not (self._db_path or self._mupp_path): - raise InvalidFileException(f"Model file '{model_path}' is invalid.") - resolved = self._db_path.resolve() - # PFS resolves a .mupp's relative DBFilePath wrongly from a forward-slash path + # PFS resolves the relative DBFilePath in a .mupp wrongly from a forward-slash path self._data_source: BaseDataSource = BaseDataSource.Create( str(resolved) if resolved.suffix.lower() == ".mupp" else resolved.as_posix() ) @@ -159,6 +153,9 @@ def create( db_sqlite = model_path.with_suffix(".sqlite") db_mupp = model_path.with_suffix(".mupp") + if projection_string and srid != -1: + raise ValueError("Projection string and SRID cannot be specified together.") + if overwrite: model_path.unlink(missing_ok=True) db_sqlite.unlink(missing_ok=True) @@ -167,9 +164,6 @@ def create( if model_path.exists() or db_sqlite.exists() or db_mupp.exists(): raise FileExistsError(f"Model file '{model_path}' already exists.") - if projection_string and srid != -1: - raise ValueError("Projection string and SRID cannot be specified together.") - try: data_source = BaseDataSource.Create(str(db_sqlite)) data_source.CreateDatabase() @@ -224,13 +218,8 @@ def open(self) -> Self: return self - def close(self) -> bool | None: - """Close the model database. - - Returns - ------- - bool or None - True if the database was already closed, otherwise None. + def close(self) -> None: + """Close the model database. Closing a closed database does nothing. Raises ------ @@ -238,7 +227,7 @@ def close(self) -> bool | None: If MIKE+ fails to close the database """ if not self._is_open: - return True + return try: self._data_table_container.UndoRedoManager.ClearUndoRedoBuffer() @@ -248,7 +237,6 @@ def close(self) -> bool | None: raise DatabaseError( f"Failed to close model database: {self._db_path}.\n{e!s}" ) - return None def ensure_mupp(self) -> Path: """Get the model's project file (.mupp), writing one if it has none. @@ -618,9 +606,7 @@ def run( Returns ------- list[Path] - Paths to the result files the simulation is configured to write. They - are not checked for existence; errors during the simulation itself - are not raised. + Paths to the result files the simulation wrote. Raises ------ @@ -628,7 +614,8 @@ def run( If `sim_option` is invalid, or not given and the active model does not determine one. RuntimeError - If the simulation engine fails to start. + If the simulation engine fails to start, exits with an error, or + doesn't write its result files. The message names the engine's logs. """ return self._runner.run(simulation_muid, sim_option=sim_option) diff --git a/mikeplus/queries.py b/mikeplus/queries.py index 2f1e222..0a4a6b4 100644 --- a/mikeplus/queries.py +++ b/mikeplus/queries.py @@ -91,7 +91,7 @@ def by_muid(self, muid_or_muids: str | list[str] | tuple[str]) -> Self: If `muid_or_muids` is an empty list/tuple. """ if isinstance(muid_or_muids, str): - self.where(f"MUID = '{muid_or_muids}'") + self.where(f"MUID = {to_sql(muid_or_muids)}") elif isinstance(muid_or_muids, (list, tuple)): if not muid_or_muids: raise ValueError( diff --git a/mikeplus/scenarios/alternative.py b/mikeplus/scenarios/alternative.py index f656229..3921d84 100644 --- a/mikeplus/scenarios/alternative.py +++ b/mikeplus/scenarios/alternative.py @@ -69,7 +69,7 @@ def __eq__(self, other: object) -> bool: """Check if two alternatives are equal.""" if not isinstance(other, Alternative): return False - return self.id == other.id + return self.id == other.id and self.group.id == other.group.id @property def id(self) -> int: @@ -146,13 +146,16 @@ def comment(self, value: str) -> None: def scenarios(self) -> list[Scenario]: """Scenarios that use this alternative.""" from .scenario import Scenario - - scenarios = [] - for scenario_id in self._scenario_manager.GetScenarios(): - scenario = self._scenario_manager.FindScenario(scenario_id) - for alt in scenario.CurAlternatives: - if alt.AltId == self._net_alternative.AltId: - scenarios.append(Scenario(self._scenario_manager, scenario)) - break - - return scenarios + from .scenario_collection import iter_net_scenarios + + # AltId is only unique within a group. + alt_id = self._net_alternative.AltId + group_id = self._net_alternative.Group.Id + return [ + Scenario(self._scenario_manager, scenario) + for scenario in iter_net_scenarios(self._scenario_manager) + if any( + alt.AltId == alt_id and alt.Group.Id == group_id + for alt in scenario.CurAlternatives + ) + ] diff --git a/mikeplus/scenarios/alternative_group.py b/mikeplus/scenarios/alternative_group.py index 483e61d..ecfcc16 100644 --- a/mikeplus/scenarios/alternative_group.py +++ b/mikeplus/scenarios/alternative_group.py @@ -95,9 +95,12 @@ def __repr__(self) -> str: return f"" def __iter__(self) -> Iterator[Alternative]: - """Iterate through all alternatives in this group.""" - yield self.base - yield from self.base.children + """Iterate through all alternatives in this group, parents before children.""" + stack = [self.base] + while stack: + alt = stack.pop() + yield alt + stack.extend(reversed(alt.children)) def _find_by_name( self, name: str, parent: IAlternative, found: list[IAlternative] diff --git a/mikeplus/scenarios/scenario_collection.py b/mikeplus/scenarios/scenario_collection.py index 2b4d332..39818c6 100644 --- a/mikeplus/scenarios/scenario_collection.py +++ b/mikeplus/scenarios/scenario_collection.py @@ -12,6 +12,16 @@ from .scenario import Scenario +def iter_net_scenarios(scenario_manager: Any) -> Iterator[Any]: + """Yield every .NET scenario, depth first from the base.""" + # GetScenarios returns names, which aren't unique, so walk the tree instead. + stack = [scenario_manager.BaseScenario] + while stack: + scenario = stack.pop() + yield scenario + stack.extend(reversed(list(scenario.Children))) + + class ScenarioCollection: """Collection-like access to scenarios in the database. @@ -25,7 +35,7 @@ class ScenarioCollection: Methods ------- find_by_name(name) : list[Scenario] - Find a scenario by name, as a list of at most one + Find all scenarios with the given name by_name(name) : Scenario or None Find first scenario that matches the given name find_by_id(id) : Scenario or None @@ -97,15 +107,12 @@ def __getitem__(self, id: str) -> Scenario: return Scenario(self._scenario_manager, scenario) def __iter__(self) -> Iterator[Scenario]: - """Iterate through all scenarios.""" - for scenario_id in self._scenario_manager.GetScenarios(): - scenario = self._scenario_manager.FindScenarioByName(scenario_id) + """Iterate through all scenarios, parents before their children.""" + for scenario in iter_net_scenarios(self._scenario_manager): yield Scenario(self._scenario_manager, scenario) def find_by_name(self, name: str) -> list[Scenario]: - """Find a scenario by name. - - Returns at most one scenario, even if several share the name. + """Find all scenarios with the given name; MIKE+ allows duplicates. Parameters ---------- @@ -115,14 +122,9 @@ def find_by_name(self, name: str) -> list[Scenario]: Returns ------- list[Scenario] - A list holding the first scenario with the given name, or an empty - list if none match + The scenarios with the given name, or an empty list if none match """ - scenario = self._scenario_manager.FindScenarioByName(name) - if scenario is None: - return [] - - return [Scenario(self._scenario_manager, scenario)] + return [scenario for scenario in self if scenario.name == name] def by_name(self, name: str) -> Scenario | None: """Find a scenario by name (returns the first match if multiple exist). diff --git a/mikeplus/simulation_runner.py b/mikeplus/simulation_runner.py index 6ff7337..51845ce 100644 --- a/mikeplus/simulation_runner.py +++ b/mikeplus/simulation_runner.py @@ -77,9 +77,7 @@ def run( Returns ------- list[Path] - Paths to the result files the simulation is configured to write. They - are not checked for existence; errors during the simulation itself - are not raised. + Paths to the result files the simulation wrote. Raises ------ @@ -87,7 +85,8 @@ def run( If `sim_option` is invalid, or not given and the active model does not determine one. RuntimeError - If the simulation engine fails to start. + If the simulation engine fails to start, exits with an error, or + doesn't write its result files. The message names the engine's logs. """ VALID_OPTIONS = ( @@ -159,7 +158,9 @@ def run_cs(self, sim_muid: str | None = None) -> list[Path]: self._handle_engine_launch(success, launcher, messages) self._wait_for_engine_completion(launcher) - return self._get_result_files("msm_Project", sim_muid) + return self._check_completed( + launcher, self._get_result_files("msm_Project", sim_muid) + ) def run_lts_joblist(self, sim_muid: str | None = None) -> list[Path]: """Run a Long Term Simulation (LTS) job list generation. @@ -185,7 +186,10 @@ def run_lts_joblist(self, sim_muid: str | None = None) -> list[Path]: self._handle_engine_launch(success, launcher, messages) self._wait_for_engine_completion(launcher) - return self._get_result_files("msm_Project", sim_muid, is_lts_joblist=True) + return self._check_completed( + launcher, + self._get_result_files("msm_Project", sim_muid, is_lts_joblist=True), + ) def run_epanet(self, sim_muid: str | None = None) -> list[Path]: """Run an EPANET water distribution simulation. @@ -215,7 +219,9 @@ def run_epanet(self, sim_muid: str | None = None) -> list[Path]: self._handle_engine_launch(success, launcher, messages) self._wait_for_engine_completion(launcher) - return self._get_result_files("mw_Project", sim_muid) + return self._check_completed( + launcher, self._get_result_files("mw_Project", sim_muid) + ) def run_swmm(self, sim_muid: str | None = None) -> list[Path]: """Run an SWMM urban drainage simulation. @@ -244,7 +250,9 @@ def run_swmm(self, sim_muid: str | None = None) -> list[Path]: self._handle_engine_launch(success, launcher, messages) self._wait_for_engine_completion(launcher) - return self._get_result_files("mss_Project", sim_muid) + return self._check_completed( + launcher, self._get_result_files("mss_Project", sim_muid) + ) def _get_sim_muid(self, sim_muid: str | None) -> str: """Get simulation MUID, or active simulation MUID if None. @@ -274,6 +282,7 @@ def _handle_engine_launch( messages_str = ". ".join(messages) if messages else "Unknown error" raise RuntimeError(f"Simulation failed to start: {messages_str}") + self._started_at = time.time() launcher.Start() def _wait_for_engine_completion(self, launcher: DhiEngineSimpleLauncher) -> None: @@ -289,6 +298,41 @@ def _wait_for_engine_completion(self, launcher: DhiEngineSimpleLauncher) -> None while launcher.IsEngineRunning: time.sleep(0.1) + def _check_completed( + self, launcher: DhiEngineSimpleLauncher, result_files: list[Path] + ) -> list[Path]: + """Return `result_files`, or raise if the engine failed. + + Returns + ------- + list[Path] + `result_files`, all of which exist. + + Raises + ------ + RuntimeError + If the engine exited with an error or a result file is missing. + + """ + missing = [path for path in result_files if not path.exists()] + if launcher.ExitCode == 0 and not missing: + return result_files + + folder = Path(self._database.db_path).parent + logs = [ + path + for pattern in ("*.log", "*ErrorLog.html") + for path in folder.rglob(pattern) + if path.stat().st_mtime >= self._started_at + ] + problem = ( + f"exited with code {launcher.ExitCode}" + if launcher.ExitCode != 0 + else f"did not write {', '.join(str(path) for path in missing)}" + ) + where = ", ".join(str(path) for path in logs) or str(folder) + raise RuntimeError(f"Simulation {problem}. See {where}.") + def _get_result_files( self, project_table_name: str, sim_muid: str, is_lts_joblist: bool = False ) -> list[Path]: diff --git a/mikeplus/tools/__init__.py b/mikeplus/tools/__init__.py index 876cd67..0335216 100644 --- a/mikeplus/tools/__init__.py +++ b/mikeplus/tools/__init__.py @@ -24,8 +24,8 @@ from .catch_slope_length_process_tool import CathSlopeLengthProcess from .connection_repair_tool import ConnectionRepairTool -from .demand_aggregation_tool import DemandAggregationTool from .create_valves_from_points_tool import CreateValvesFromPointsTool +from .demand_aggregation_tool import DemandAggregationTool from .demand_connection_tool import DemandConnectionTool from .import_tool import ImportTool from .interpolation_tool import InterpolationTool @@ -34,8 +34,8 @@ __all__ = [ "CathSlopeLengthProcess", "ConnectionRepairTool", - "DemandAggregationTool", "CreateValvesFromPointsTool", + "DemandAggregationTool", "DemandConnectionTool", "ImportTool", "InterpolationTool", diff --git a/mikeplus/tools/catch_slope_length_process_tool.py b/mikeplus/tools/catch_slope_length_process_tool.py index f31a3ba..4891ad7 100644 --- a/mikeplus/tools/catch_slope_length_process_tool.py +++ b/mikeplus/tools/catch_slope_length_process_tool.py @@ -3,6 +3,7 @@ from __future__ import annotations import os.path +import warnings from collections.abc import Iterable from typing import TYPE_CHECKING, Any @@ -78,7 +79,8 @@ def run( dem_layer = os.path.abspath(dem_layer) unit = eumUnit(demUnitKey) tool = CatchmentSlope(self._dataTables) - warnings = List[str]() + tool.RuningProgress += self._on_tool_runing_progress + tool_warnings = List[str]() catch_list = List[str]() for selCatch in catch_ids: catch_list.Add(selCatch) @@ -91,9 +93,10 @@ def run( dem_layer, 1, unit, - warnings, + tool_warnings, ) - tool.RuningProgress += self._on_tool_runing_progress + for message in tool_warnings: + warnings.warn(str(message), stacklevel=2) def _on_tool_runing_progress(self, source: Any, args: Any) -> None: print(args.Msg) diff --git a/mikeplus/tools/connection_repair_tool.py b/mikeplus/tools/connection_repair_tool.py index f6f4f45..b2f524b 100644 --- a/mikeplus/tools/connection_repair_tool.py +++ b/mikeplus/tools/connection_repair_tool.py @@ -35,8 +35,8 @@ def __init__(self, database): def run(self): """Run the connection repair tool.""" tool = ConnectionRepairEngine(self._dataTables) - tool.Run() tool.RuningProgress += self._on_tool_runing_progress + tool.Run() def _on_tool_runing_progress(self, source, args): print(args.Msg) diff --git a/mikeplus/tools/create_valves_from_points_tool.py b/mikeplus/tools/create_valves_from_points_tool.py index 0092dbb..84a9863 100644 --- a/mikeplus/tools/create_valves_from_points_tool.py +++ b/mikeplus/tools/create_valves_from_points_tool.py @@ -26,7 +26,7 @@ class CreateValvesFromPointsTool: Reads a point shapefile and adds one valve to `mw_Valve` per point. A point within the search radius of a junction with one or two pipes gets a valve at that junction: the valve takes the first `valve_length` of a pipe and a - new junction joins the two. Otherwise the nearest pipe within the search + new junction joins the two. Otherwise, the nearest pipe within the search radius is split where the point projects onto it, and the valve takes `valve_length` centred there. Points that can't be placed are skipped with a message. diff --git a/mikeplus/tools/demand_aggregation_tool.py b/mikeplus/tools/demand_aggregation_tool.py index 9a4c3dd..63c721d 100644 --- a/mikeplus/tools/demand_aggregation_tool.py +++ b/mikeplus/tools/demand_aggregation_tool.py @@ -127,7 +127,7 @@ def assign_to_multiple_demands( Unlike `aggregate_to_node_demands`, nothing is summed: every allocation becomes one junction demand, keeping its demand, category, - pattern and meter. + pattern, and meter. Parameters ---------- diff --git a/mikeplus/tools/demand_connection_tool.py b/mikeplus/tools/demand_connection_tool.py index 628c19d..c80ca2e 100644 --- a/mikeplus/tools/demand_connection_tool.py +++ b/mikeplus/tools/demand_connection_tool.py @@ -55,7 +55,7 @@ class DemandConnectionTool: Examples -------- - Connect every demand allocation to the nearest junction within 50 m. + Connect every demand allocation to the nearest junction within 50 metres. ```python >>> from mikeplus import Database diff --git a/mikeplus/tools/topology_repair_tool.py b/mikeplus/tools/topology_repair_tool.py index f653d4c..643dda5 100644 --- a/mikeplus/tools/topology_repair_tool.py +++ b/mikeplus/tools/topology_repair_tool.py @@ -81,11 +81,21 @@ def run( snap_distance : float, optional The distance used in dissolve the overlapped nodes, in correct link connection, and in split link on T junction, by default 0.1. - Notes - ----- - Does nothing unless the active model is CS_MIKE1D or WD_EPANET. + Raises + ------ + ValueError + If the active model is neither CS_MIKE1D nor WD_EPANET. """ + active_model = self._dataTables.ActiveModel + if active_model == MUModelOption.CS_MIKE1D: + tool = CSTopologyRepairTool(self._dataTables) + elif active_model == MUModelOption.WD_EPANET: + tool = WDTopologyRepairTool(self._dataTables) + else: + raise ValueError( + f"Topology repair needs a CS_MIKE1D or WD_EPANET model, not {active_model}." + ) cancel_source = CancellationTokenSource() topology_param = TopologyRepairParam() topology_param.DeleteUnLinkNodeAndLink = delete_unLink_node_Link @@ -98,14 +108,8 @@ def run( topology_param.OverlapNodeSearchRadius = snap_distance topology_param.JunctionSearchRadius4Connection = snap_distance topology_param.TJunctionSplitSearchRadius = snap_distance - if self._dataTables.ActiveModel == MUModelOption.CS_MIKE1D: - tool = CSTopologyRepairTool(self._dataTables) - tool.RuningProgress += self._on_tool_runing_progress - tool.Run(topology_param, cancel_source.Token, False) - elif self._dataTables.ActiveModel == MUModelOption.WD_EPANET: - tool = WDTopologyRepairTool(self._dataTables) - tool.RuningProgress += self._on_tool_runing_progress - tool.Run(topology_param, cancel_source.Token, False) + tool.RuningProgress += self._on_tool_runing_progress + tool.Run(topology_param, cancel_source.Token, False) def _on_tool_runing_progress(self, source: Any, args: Any) -> None: print(args.Msg) diff --git a/mikeplus/utils.py b/mikeplus/utils.py index 99ba65d..88c04e1 100644 --- a/mikeplus/utils.py +++ b/mikeplus/utils.py @@ -189,7 +189,8 @@ def to_sql(value: Any) -> str: Returns ------- str - The SQL string representation of the value. + The SQL string representation of the value. Booleans become `1` or `0`, + and single quotes in strings are doubled. Examples -------- @@ -199,15 +200,20 @@ def to_sql(value: Any) -> str: '10.5' >>> to_sql("test_muid") "'test_muid'" + >>> to_sql(True) + '1' >>> to_sql(None) 'NULL' """ if value is None: return "NULL" + elif isinstance(value, bool): + return str(int(value)) elif isinstance(value, (int, float)): return str(value) else: - return f"'{value!s}'" + escaped = str(value).replace("'", "''") + return f"'{escaped}'" __all__ = ["to_sql"] diff --git a/scripts/docs.py b/scripts/docs.py index 978fd48..c29f6d9 100644 --- a/scripts/docs.py +++ b/scripts/docs.py @@ -22,6 +22,7 @@ # `mikeplus` and `min_slope`, doctest ellipses, `.NET`, `TODO(2027)` tags. IGNORED_PROOFREAD_RULES = [ "AnA", + "ExpandAlloc", "ExpandConfiguration", "ExpandMinimum", "ExpandTimeShorthands", diff --git a/tests/database/test_database.py b/tests/database/test_database.py index c15c688..910c70d 100644 --- a/tests/database/test_database.py +++ b/tests/database/test_database.py @@ -74,6 +74,13 @@ def test_create_existing_model_raises_error(self, session_sirius_db: Path): with pytest.raises(FileExistsError): Database.create(session_sirius_db) + def test_create_overwrite_with_invalid_args_keeps_file(self, tmp_path: Path): + existing = tmp_path / "existing.sqlite" + existing.write_bytes(b"keep me") + with pytest.raises(ValueError, match="cannot be specified together"): + Database.create(existing, srid=4326, projection_string="x", overwrite=True) + assert existing.read_bytes() == b"keep me" + def test_import_epanet(self, tmp_path: Path): """Test importing from an EPANET .inp file.""" db_path = tmp_path / "model.sqlite" diff --git a/tests/database/test_queries.py b/tests/database/test_queries.py index c3d5e0c..ab5a141 100644 --- a/tests/database/test_queries.py +++ b/tests/database/test_queries.py @@ -73,6 +73,7 @@ def test_execute(self, base_query: BaseQueryTest): "muid, expected_conditions", [ ("test_muid_1", ["MUID = 'test_muid_1'"]), + ("O'Brien", ["MUID = 'O''Brien'"]), (["muid_A", "muid_B"], ["MUID IN ('muid_A', 'muid_B')"]), (["muid_X", "muid_Y"], ["MUID IN ('muid_X', 'muid_Y')"]), ([], ValueError), diff --git a/tests/test_scenarios.py b/tests/test_scenarios.py index 37c5f89..551e42d 100644 --- a/tests/test_scenarios.py +++ b/tests/test_scenarios.py @@ -121,3 +121,36 @@ def test_api_create_and_activate_scenario(sirius_db): assert found_scenario is not None assert found_scenario.id == new_scenario.id + +def test_duplicate_scenario_names_are_distinct(sirius_db): + with mp.open(sirius_db) as db: + first = db.scenarios.create("dup") + second = db.scenarios.create("dup") + ids = [scenario.id for scenario in db.scenarios] + assert len(ids) == len(set(ids)) + assert {s.id for s in db.scenarios.find_by_name("dup")} == {first.id, second.id} + + +def test_alternative_scenarios_with_child_scenario(sirius_db): + with mp.open(sirius_db) as db: + group = db.alternative_groups["Catchments and hydrology data"] + alt = group.create("child alt") + scenario = db.scenarios.create("uses child alt") + scenario.set_alternative(alt) + assert [s.id for s in alt.scenarios] == [scenario.id] + assert scenario.id not in [s.id for s in group.base.scenarios] + + +def test_base_alternatives_of_different_groups_differ(session_sirius_db): + with mp.open(session_sirius_db) as db: + first, second = list(db.alternative_groups)[:2] + assert first.base != second.base + assert first.base == first.base + + +def test_alternative_group_iterates_grandchildren(sirius_db): + with mp.open(sirius_db) as db: + group = db.alternative_groups["Catchments and hydrology data"] + child = group.create("child") + grandchild = group.create("grandchild", parent=child) + assert grandchild in list(group) diff --git a/tests/test_simulation_runner.py b/tests/test_simulation_runner.py index 4e92a87..35d3adf 100644 --- a/tests/test_simulation_runner.py +++ b/tests/test_simulation_runner.py @@ -112,4 +112,11 @@ def test_mike_engine_run_epanet(epanet_demo_db): file.unlink(missing_ok=True) db.close() - + + +@pytest.mark.slow +def test_run_raises_when_simulation_fails(sirius_db): + with Database(sirius_db) as db: + db.tables.msm_Link.update({"Diameter": 0}).all().execute() + with pytest.raises(RuntimeError, match="exited with code"): + db.run() diff --git a/tests/test_tools.py b/tests/test_tools.py index f2a1f3e..51dd55a 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -257,3 +257,8 @@ def test_import_tool(import_db): muids = db.tables.msm_Link.get_muids() assert len(muids) == 575 db.close() + + +def test_topology_repair_rejects_unsupported_model(swmm_db): + with Database(swmm_db) as db, pytest.raises(ValueError, match="CS_MIKE1D"): + TopoRepairTool(db).run() diff --git a/tests/test_utils.py b/tests/test_utils.py index 248a7b1..3764e7a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,9 +2,23 @@ import pytest -from mikeplus.utils import _try_setup_default_bin_path +from mikeplus.utils import _try_setup_default_bin_path, to_sql def test_no_default_install_root_names_env_var(): with pytest.raises(FileNotFoundError, match="MIKEPLUSPY_INSTALL_ROOT"): _try_setup_default_bin_path(None, Path("bin/x64"), "MIKEPLUSPY_INSTALL_ROOT") + + +@pytest.mark.parametrize( + "value, expected", + [ + ("O'Brien", "'O''Brien'"), + (True, "1"), + (False, "0"), + (None, "NULL"), + (2.5, "2.5"), + ], +) +def test_to_sql(value, expected): + assert to_sql(value) == expected