From fb7981977c48d9afeb21f09f6115ff2fc0a228af Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Wed, 12 Aug 2026 17:32:26 -0600 Subject: [PATCH 01/10] feat: implement update logic for properties in plexos system --- src/plexosdb/db.py | 119 ++++++++++++++++++++++++- tests/test_plexosdb_update_property.py | 93 +++++++++++++++++++ 2 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 tests/test_plexosdb_update_property.py diff --git a/src/plexosdb/db.py b/src/plexosdb/db.py index a50577a..434d1b9 100644 --- a/src/plexosdb/db.py +++ b/src/plexosdb/db.py @@ -4334,7 +4334,18 @@ def update_object( def update_properties(self, updates: list[dict[str, Any]]) -> None: """Update multiple properties in a single transaction.""" - raise NotImplementedError # pragma: no cover + with self._db.transaction(): + for update in updates: + self._update_property_value( + update["object_name"], + update["property_name"], + update["new_value"], + object_class=update["object_class"], + scenario=update.get("scenario"), + band=update.get("band"), + collection=update.get("collection"), + parent_class=update.get("parent_class"), + ) def update_property( self, @@ -4350,7 +4361,111 @@ def update_property( parent_class: ClassEnum | None = None, ) -> None: """Update a property value for a given object.""" - raise NotImplementedError # pragma: no cover + self.update_properties( + [ + { + "object_name": object_name, + "property_name": property_name, + "new_value": new_value, + "object_class": object_class, + "scenario": scenario, + "band": band, + "collection": collection, + "parent_class": parent_class, + } + ] + ) + + def _update_property_value( + self, + object_name: str, + property_name: str, + new_value: str | None, + /, + *, + object_class: ClassEnum, + scenario: str | None, + band: str | int | None, + collection: CollectionEnum | None, + parent_class: ClassEnum | None, + ) -> None: + """Update matching property rows inside an active transaction.""" + if not checks_module.check_object_exists(self, object_class, object_name): + raise NotFoundError(f"Object = `{object_name}` does not exist for class `{object_class}`.") + + collection = collection or get_default_collection(object_class) + parent_class = parent_class or ClassEnum.System + valid_properties = self.list_valid_properties( + collection, + parent_class_enum=parent_class, + child_class_enum=object_class, + ) + if property_name not in valid_properties: + raise NameError(f"Property {property_name} does not exist for collection: {collection}.") + + membership_id = resolve_membership_id( + self, + object_name, + object_class=object_class, + collection=collection, + parent_class=parent_class, + ) + property_id = self.get_property_id( + property_name, + collection_enum=collection, + child_class_enum=object_class, + parent_class_enum=parent_class, + ) + + conditions = ["d.membership_id = ?", "d.property_id = ?"] + params: list[Any] = [membership_id, property_id] + + scenario_class_id = self.get_class_id(ClassEnum.Scenario) + if scenario is None: + conditions.append( + "NOT EXISTS (" + "SELECT 1 FROM t_tag scenario_tag " + "JOIN t_object scenario_object ON scenario_object.object_id = scenario_tag.object_id " + "WHERE scenario_tag.data_id = d.data_id " + "AND scenario_object.class_id = ?" + ")" + ) + params.append(scenario_class_id) + else: + scenario_id = self.get_scenario_id(scenario) + conditions.append( + "EXISTS (SELECT 1 FROM t_tag scenario_tag " + "WHERE scenario_tag.data_id = d.data_id " + "AND scenario_tag.object_id = ?)" + ) + params.append(scenario_id) + + if band is not None: + try: + band_id = int(band) + except (TypeError, ValueError) as exc: + raise ValueError(f"Band must be an integer, got {band!r}.") from exc + conditions.append( + "EXISTS (SELECT 1 FROM t_band property_band " + "WHERE property_band.data_id = d.data_id AND property_band.band_id = ?)" + ) + params.append(band_id) + + where_clause = " AND ".join(conditions) + data_ids = self._db.fetchall(f"SELECT d.data_id FROM t_data d WHERE {where_clause}", tuple(params)) + if not data_ids: + scenario_detail = f" for scenario `{scenario}`" if scenario is not None else "" + band_detail = f" and band `{band}`" if band is not None else "" + raise NotFoundError( + f"Property `{property_name}` was not found for object `{object_name}`" + f"{scenario_detail}{band_detail}." + ) + + placeholders = ", ".join("?" for _ in data_ids) + self._db.execute( + f"UPDATE t_data SET value = ? WHERE data_id IN ({placeholders})", + (new_value, *(row[0] for row in data_ids)), + ) def update_scenario( self, diff --git a/tests/test_plexosdb_update_property.py b/tests/test_plexosdb_update_property.py new file mode 100644 index 0000000..ce3f319 --- /dev/null +++ b/tests/test_plexosdb_update_property.py @@ -0,0 +1,93 @@ +from pathlib import Path + +import pytest + +from plexosdb import ClassEnum, PlexosDB +from plexosdb.exceptions import NotFoundError + + +XML_PATH = Path(__file__).parent / "data" / "run_of_river_case" / "TestSystem.xml" + + +@pytest.fixture +def run_of_river_db(): + db = PlexosDB.from_xml(XML_PATH) + yield db + db._db.close() + + +def _property_rows(db: PlexosDB, object_name: str, property_name: str) -> list[tuple]: + return db.query( + """ + SELECT d.value, COALESCE(b.band_id, 1) + FROM t_data AS d + JOIN t_membership AS m ON m.membership_id = d.membership_id + JOIN t_object AS o ON o.object_id = m.child_object_id + JOIN t_property AS p ON p.property_id = d.property_id + LEFT JOIN t_band AS b ON b.data_id = d.data_id + WHERE o.name = ? AND p.name = ? + ORDER BY COALESCE(b.band_id, 1) + """, + (object_name, property_name), + ) + + +def test_update_property_updates_xml_fixture_value(run_of_river_db: PlexosDB) -> None: + run_of_river_db.update_property( + "Coal_Gen", + "Max Capacity", + 625.0, + object_class=ClassEnum.Generator, + ) + + rows = _property_rows(run_of_river_db, "Coal_Gen", "Max Capacity") + assert rows == [(625.0, 1)] + + +def test_update_property_only_updates_requested_band(run_of_river_db: PlexosDB) -> None: + run_of_river_db.update_property( + "Gas_Gen2", + "Load Point", + 95.0, + object_class=ClassEnum.Generator, + band=2, + ) + + rows = _property_rows(run_of_river_db, "Gas_Gen2", "Load Point") + assert rows == [ + (95.0 if band == 2 else 50.0 + 20.0 * band, band) + for band in range(1, 11) + ] + + +def test_update_properties_updates_multiple_xml_fixture_values(run_of_river_db: PlexosDB) -> None: + run_of_river_db.update_properties( + [ + { + "object_name": "Coal_Gen", + "property_name": "Max Capacity", + "new_value": 625.0, + "object_class": ClassEnum.Generator, + }, + { + "object_name": "Gas_Gen1", + "property_name": "Max Capacity", + "new_value": 325.0, + "object_class": ClassEnum.Generator, + }, + ] + ) + + assert _property_rows(run_of_river_db, "Coal_Gen", "Max Capacity") == [(625.0, 1)] + assert _property_rows(run_of_river_db, "Gas_Gen1", "Max Capacity") == [(325.0, 1)] + + +def test_update_property_raises_for_missing_fixture_property(run_of_river_db: PlexosDB) -> None: + with pytest.raises(NotFoundError): + run_of_river_db.update_property( + "Coal_Gen", + "Max Capacity", + 625.0, + object_class=ClassEnum.Generator, + band=99, + ) \ No newline at end of file From e78862aca5961f2b2c4c9457068493cbeffe13f0 Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Wed, 12 Aug 2026 17:32:54 -0600 Subject: [PATCH 02/10] docs: add corresponding definition and examples for new functions --- docs/source/howtos/add_properties.md | 47 +++++++++++++++++++++++++++ docs/source/howtos/bulk_operations.md | 34 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/docs/source/howtos/add_properties.md b/docs/source/howtos/add_properties.md index c3f60c8..def1aa2 100644 --- a/docs/source/howtos/add_properties.md +++ b/docs/source/howtos/add_properties.md @@ -81,6 +81,53 @@ db.add_property( ) ``` +## Updating Properties + +Use `update_property` to change the value of an existing property without +removing its scenario, band, date, or text metadata: + +```python +db.update_property( + "Generator1", + "Max Capacity", + 125.0, + object_class=ClassEnum.Generator, +) +``` + +When a property has multiple bands, pass `band` to update only the matching +band. Pass `scenario` to update a scenario-specific value. If `scenario` is +omitted, only the base, non-scenario property is updated. + +```python +db.update_property( + "Generator1", + "Heat Rate", + 9.8, + object_class=ClassEnum.Generator, + band=2, + scenario="High Demand", +) +``` + +The `collection` and `parent_class` arguments can be supplied when the property +belongs to a non-default collection or membership: + +```python +db.update_property( + "Generator1", + "Max Capacity", + 130.0, + object_class=ClassEnum.Generator, + collection=CollectionEnum.Generators, + parent_class=ClassEnum.System, +) +``` + +The method raises `NotFoundError` when the object or matching property row does +not exist, and `NameError` when the property is invalid for the selected +collection. + ## Bulk Adding Properties For efficiency when adding many properties at once (use the flat format; the diff --git a/docs/source/howtos/bulk_operations.md b/docs/source/howtos/bulk_operations.md index 5cad05f..fd679ca 100644 --- a/docs/source/howtos/bulk_operations.md +++ b/docs/source/howtos/bulk_operations.md @@ -70,6 +70,40 @@ Key performance features: - Direct SQL execution with prepared statements - Automatic property enablement (sets `is_dynamic` and `is_enabled` flags) +## Bulk Updating Properties + +Use `update_properties` to update several existing property values in one +transaction. Each update record uses the same selectors as `update_property`: +`object_name`, `property_name`, `new_value`, and `object_class`. The optional +`scenario`, `band`, `collection`, and `parent_class` fields narrow the matching +property row. + +```python +updates = [ + { + "object_name": "Generator1", + "property_name": "Max Capacity", + "new_value": 125.0, + "object_class": ClassEnum.Generator, + }, + { + "object_name": "Generator2", + "property_name": "Heat Rate", + "new_value": 9.8, + "object_class": ClassEnum.Generator, + "band": 2, + "scenario": "High Demand", + }, +] + +db.update_properties(updates) +``` + +All updates are committed together. If an update cannot find its object, +property, scenario, or requested band, the transaction fails and earlier updates +in the batch are rolled back. When `scenario` is omitted, the update targets the +base property rather than a scenario-tagged row. + ### Handling Different Object Classes You can process different types of objects separately: From b195558a95c7ab35744018bca8711cc4d494de68 Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Wed, 12 Aug 2026 17:40:04 -0600 Subject: [PATCH 03/10] fix: resolve prek issues --- tests/test_plexosdb_update_property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_plexosdb_update_property.py b/tests/test_plexosdb_update_property.py index ce3f319..6e7d0be 100644 --- a/tests/test_plexosdb_update_property.py +++ b/tests/test_plexosdb_update_property.py @@ -90,4 +90,4 @@ def test_update_property_raises_for_missing_fixture_property(run_of_river_db: Pl 625.0, object_class=ClassEnum.Generator, band=99, - ) \ No newline at end of file + ) From 8b35fd34d47722d61ac459ff0f418dcf9069d3fb Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 11:24:42 -0600 Subject: [PATCH 04/10] fix: address roll back domain-validation failures --- src/plexosdb/db_manager.py | 6 ++-- tests/test_plexosdb_update_property.py | 41 ++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/plexosdb/db_manager.py b/src/plexosdb/db_manager.py index 7dbca7e..ab7bde0 100644 --- a/src/plexosdb/db_manager.py +++ b/src/plexosdb/db_manager.py @@ -818,13 +818,13 @@ def transaction(self) -> Generator["SQLiteManager", None, None]: Raises ------ - sqlite3.Error - If a database error occurs during transaction + Exception + Re-raises any exception from the transaction after rolling back """ try: self.connection.execute("BEGIN") yield self - except sqlite3.Error: + except Exception: self.connection.rollback() raise else: diff --git a/tests/test_plexosdb_update_property.py b/tests/test_plexosdb_update_property.py index 6e7d0be..8778db4 100644 --- a/tests/test_plexosdb_update_property.py +++ b/tests/test_plexosdb_update_property.py @@ -3,7 +3,7 @@ import pytest from plexosdb import ClassEnum, PlexosDB -from plexosdb.exceptions import NotFoundError +from plexosdb.exceptions import NameError, NotFoundError XML_PATH = Path(__file__).parent / "data" / "run_of_river_case" / "TestSystem.xml" @@ -54,10 +54,7 @@ def test_update_property_only_updates_requested_band(run_of_river_db: PlexosDB) ) rows = _property_rows(run_of_river_db, "Gas_Gen2", "Load Point") - assert rows == [ - (95.0 if band == 2 else 50.0 + 20.0 * band, band) - for band in range(1, 11) - ] + assert rows == [(95.0 if band == 2 else 50.0 + 20.0 * band, band) for band in range(1, 11)] def test_update_properties_updates_multiple_xml_fixture_values(run_of_river_db: PlexosDB) -> None: @@ -82,6 +79,40 @@ def test_update_properties_updates_multiple_xml_fixture_values(run_of_river_db: assert _property_rows(run_of_river_db, "Gas_Gen1", "Max Capacity") == [(325.0, 1)] +def test_update_properties_rolls_back_domain_validation_failure_and_reuses_connection( + run_of_river_db: PlexosDB, +) -> None: + original_rows = _property_rows(run_of_river_db, "Coal_Gen", "Max Capacity") + + with pytest.raises(NameError): + run_of_river_db.update_properties( + [ + { + "object_name": "Coal_Gen", + "property_name": "Max Capacity", + "new_value": 625.0, + "object_class": ClassEnum.Generator, + }, + { + "object_name": "Coal_Gen", + "property_name": "Not a property", + "new_value": 1.0, + "object_class": ClassEnum.Generator, + }, + ] + ) + + assert _property_rows(run_of_river_db, "Coal_Gen", "Max Capacity") == original_rows + + run_of_river_db.update_property( + "Coal_Gen", + "Max Capacity", + 625.0, + object_class=ClassEnum.Generator, + ) + assert _property_rows(run_of_river_db, "Coal_Gen", "Max Capacity") == [(625.0, 1)] + + def test_update_property_raises_for_missing_fixture_property(run_of_river_db: PlexosDB) -> None: with pytest.raises(NotFoundError): run_of_river_db.update_property( From c5e4098bcb2edb13f735b05d7bd94f9ac76bc2ec Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 11:35:15 -0600 Subject: [PATCH 05/10] fix: avoid n+1 lookups in bulk updates --- src/plexosdb/db.py | 200 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 190 insertions(+), 10 deletions(-) diff --git a/src/plexosdb/db.py b/src/plexosdb/db.py index 434d1b9..71fb77e 100644 --- a/src/plexosdb/db.py +++ b/src/plexosdb/db.py @@ -39,6 +39,7 @@ no_space, normalize_names, plan_property_inserts, + _resolve_membership_map, resolve_membership_id, _normalize_attribute_records, ) @@ -4334,18 +4335,197 @@ def update_object( def update_properties(self, updates: list[dict[str, Any]]) -> None: """Update multiple properties in a single transaction.""" + if not updates: + return + + prepared_updates = self._prepare_property_updates(updates) + with self._db.transaction(): - for update in updates: - self._update_property_value( - update["object_name"], - update["property_name"], - update["new_value"], - object_class=update["object_class"], - scenario=update.get("scenario"), - band=update.get("band"), - collection=update.get("collection"), - parent_class=update.get("parent_class"), + self._db.executemany("UPDATE t_data SET value = ? WHERE data_id = ?", prepared_updates) + + def _prepare_property_updates(self, updates: list[dict[str, Any]]) -> list[tuple[Any, int]]: + grouped_updates: dict[tuple[ClassEnum, CollectionEnum, ClassEnum], list[dict[str, Any]]] = {} + for update in updates: + object_class = update["object_class"] + key = ( + object_class, + update.get("collection") or get_default_collection(object_class), + update.get("parent_class") or ClassEnum.System, + ) + grouped_updates.setdefault(key, []).append(update) + + scenario_class_id = self.get_class_id(ClassEnum.Scenario) + prepared_updates: list[tuple[Any, int]] = [] + for (object_class, collection, parent_class), group in grouped_updates.items(): + prepared_updates.extend( + self._prepare_property_update_group( + group, + object_class=object_class, + collection=collection, + parent_class=parent_class, + scenario_class_id=scenario_class_id, + ) + ) + return prepared_updates + + def _prepare_property_update_group( + self, + group: list[dict[str, Any]], + *, + object_class: ClassEnum, + collection: CollectionEnum, + parent_class: ClassEnum, + scenario_class_id: int, + ) -> list[tuple[Any, int]]: + object_names = tuple({update["object_name"] for update in group}) + object_class_id = self.get_class_id(object_class) + object_placeholders = ", ".join("?" for _ in object_names) + object_rows = self._db.fetchall( + f"SELECT name FROM t_object WHERE class_id = ? AND name IN ({object_placeholders})", + (object_class_id, *object_names), + ) + existing_object_names = {row[0] for row in object_rows} + missing_object_names = [name for name in object_names if name not in existing_object_names] + if missing_object_names: + raise NotFoundError( + f"Object = `{missing_object_names[0]}` does not exist for class `{object_class}`." + ) + + valid_properties = self.list_valid_properties( + collection, + parent_class_enum=parent_class, + child_class_enum=object_class, + ) + collection_id = self.get_collection_id( + collection, + parent_class_enum=parent_class, + child_class_enum=object_class, + ) + property_rows = self._db.fetchall( + "SELECT name, property_id FROM t_property WHERE collection_id = ?", + (collection_id,), + ) + property_ids = {name: property_id for name, property_id in property_rows} + membership_map = _resolve_membership_map( + self, + [{"name": update["object_name"]} for update in group], + object_class=object_class, + parent_class=parent_class, + collection=collection, + ) + scenario_ids = self._resolve_scenario_ids(group, scenario_class_id) + selectors = self._prepare_property_update_selectors( + group, + valid_properties=valid_properties, + property_ids=property_ids, + membership_map=membership_map, + scenario_ids=scenario_ids, + collection=collection, + ) + data_ids_by_index = self._find_property_data_ids(selectors, scenario_class_id) + + prepared_updates: list[tuple[Any, int]] = [] + for index, update in enumerate(group): + data_ids = data_ids_by_index.get(index, []) + if not data_ids: + scenario = update.get("scenario") + band = update.get("band") + scenario_detail = f" for scenario `{scenario}`" if scenario is not None else "" + band_detail = f" and band `{band}`" if band is not None else "" + raise NotFoundError( + f"Property `{update['property_name']}` was not found for object `" + f"{update['object_name']}`{scenario_detail}{band_detail}." ) + prepared_updates.extend((update["new_value"], data_id) for data_id in data_ids) + return prepared_updates + + def _resolve_scenario_ids(self, group: list[dict[str, Any]], scenario_class_id: int) -> dict[str, int]: + scenario_names = tuple({update["scenario"] for update in group if update.get("scenario") is not None}) + if not scenario_names: + return {} + placeholders = ", ".join("?" for _ in scenario_names) + rows = self._db.fetchall( + f"SELECT object_id, name FROM t_object WHERE class_id = ? AND name IN ({placeholders})", + (scenario_class_id, *scenario_names), + ) + return {name: scenario_id for scenario_id, name in rows} + + def _prepare_property_update_selectors( + self, + group: list[dict[str, Any]], + *, + valid_properties: list[str], + property_ids: dict[str, int], + membership_map: dict[str, int], + scenario_ids: dict[str, int], + collection: CollectionEnum, + ) -> list[tuple[int, int, int, int | None, int | None]]: + selectors: list[tuple[int, int, int, int | None, int | None]] = [] + for index, update in enumerate(group): + property_name = update["property_name"] + if property_name not in valid_properties: + raise NameError(f"Property {property_name} does not exist for collection: {collection}.") + scenario = update.get("scenario") + scenario_id = scenario_ids.get(scenario) if scenario is not None else None + if scenario is not None and scenario_id is None: + raise AssertionError(f"Scenario {scenario!r} does not exist.") + band = update.get("band") + try: + band_id = int(band) if band is not None else None + except (TypeError, ValueError) as exc: + raise ValueError(f"Band must be an integer, got {band!r}.") from exc + selectors.append( + ( + index, + membership_map[update["object_name"]], + property_ids[property_name], + scenario_id, + band_id, + ) + ) + return selectors + + def _find_property_data_ids( + self, + selectors: list[tuple[int, int, int, int | None, int | None]], + scenario_class_id: int, + ) -> dict[int, list[int]]: + data_ids_by_index: dict[int, list[int]] = {} + for selector_batch in batched(selectors, 180): + values_sql = ", ".join("(?, ?, ?, ?, ?)" for _ in selector_batch) + selector_params = [value for selector in selector_batch for value in selector] + data_rows = self._db.fetchall( + f""" + WITH update_rows(update_index, membership_id, property_id, scenario_id, band_id) AS ( + VALUES {values_sql} + ) + SELECT update_rows.update_index, d.data_id + FROM update_rows + JOIN t_data AS d + ON d.membership_id = update_rows.membership_id + AND d.property_id = update_rows.property_id + WHERE ( + (update_rows.scenario_id IS NULL AND NOT EXISTS ( + SELECT 1 FROM t_tag AS scenario_tag + JOIN t_object AS scenario_object ON scenario_object.object_id = scenario_tag.object_id + WHERE scenario_tag.data_id = d.data_id AND scenario_object.class_id = ? + )) + OR (update_rows.scenario_id IS NOT NULL AND EXISTS ( + SELECT 1 FROM t_tag AS scenario_tag + WHERE scenario_tag.data_id = d.data_id + AND scenario_tag.object_id = update_rows.scenario_id + )) + ) + AND (update_rows.band_id IS NULL OR EXISTS ( + SELECT 1 FROM t_band AS property_band + WHERE property_band.data_id = d.data_id AND property_band.band_id = update_rows.band_id + )) + """, + (*selector_params, scenario_class_id), + ) + for index, data_id in data_rows: + data_ids_by_index.setdefault(index, []).append(data_id) + return data_ids_by_index def update_property( self, From 436daba4048fa2b3d8d00cbd2e913c467425595e Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 12:01:44 -0600 Subject: [PATCH 06/10] fix: reuse shared property-context for crud operations --- src/plexosdb/db.py | 128 +++++++++++------------------- tests/test_plexosdb_properties.py | 24 +++++- 2 files changed, 70 insertions(+), 82 deletions(-) diff --git a/src/plexosdb/db.py b/src/plexosdb/db.py index 71fb77e..a6603d6 100644 --- a/src/plexosdb/db.py +++ b/src/plexosdb/db.py @@ -1176,6 +1176,47 @@ def _handle_dates( (data_id, date_to.isoformat()), ) + def _resolve_property_context( + self, + object_name: str, + property_name: str, + /, + *, + object_class: ClassEnum, + collection: CollectionEnum | None = None, + parent_class: ClassEnum | None = None, + parent_object_name: str | None = None, + ) -> tuple[CollectionEnum, ClassEnum, int, int]: + """Resolve the shared object, property, and membership context.""" + if not checks_module.check_object_exists(self, object_class, object_name): + raise NotFoundError(f"Object = `{object_name}` does not exist for class `{object_class}`.") + + collection = collection or get_default_collection(object_class) + parent_class = parent_class or ClassEnum.System + valid_properties = self.list_valid_properties( + collection, + parent_class_enum=parent_class, + child_class_enum=object_class, + ) + if property_name not in valid_properties: + raise NameError(f"Property '{property_name}' does not exist for collection: {collection}.") + + property_id = self.get_property_id( + property_name, + collection_enum=collection, + child_class_enum=object_class, + parent_class_enum=parent_class, + ) + membership_id = resolve_membership_id( + self, + object_name, + object_class=object_class, + collection=collection, + parent_class=parent_class, + parent_object_name=parent_object_name, + ) + return collection, parent_class, property_id, membership_id + def add_property( self, object_class_enum: ClassEnum, @@ -1267,37 +1308,9 @@ def add_property( >>> db.add_property(ClassEnum.Generator, "Generator1", "Max Capacity", 100.0) 1 """ - # Ensure object exist - if not checks_module.check_object_exists(self, object_class_enum, object_name): - msg = f"Object = `{object_name}` does not exist on the system. " - f"Check available objects for class `{object_class_enum}` using `list_objects_by_class`" - raise NotFoundError(msg) - _ = self.get_object_id(object_class_enum, object_name) - - if not collection_enum: - collection_enum = get_default_collection(object_class_enum) - - parent_class_enum = parent_class_enum or ClassEnum.System - - valid_properties = self.list_valid_properties( - collection_enum, child_class_enum=object_class_enum, parent_class_enum=parent_class_enum - ) - if name not in valid_properties: - msg = ( - f"Property {name} does not exist for collection: {collection_enum}. " - f"Run `self.list_valid_properties({collection_enum}) to verify valid properties." - ) - raise NameError(msg) - - property_id = self.get_property_id( - name, - parent_class_enum=parent_class_enum, - child_class_enum=object_class_enum, - collection_enum=collection_enum, - ) - membership_id = resolve_membership_id( - self, + collection_enum, parent_class_enum, property_id, membership_id = self._resolve_property_context( object_name, + name, object_class=object_class_enum, collection=collection_enum, parent_class=parent_class_enum, @@ -2130,37 +2143,9 @@ def delete_property( >>> db.add_property(ClassEnum.Generator, "Generator1", "Max Capacity", 100.0) >>> db.delete_property(ClassEnum.Generator, "Generator1", property_name="Max Capacity") """ - # Ensure object exists - if not checks_module.check_object_exists(self, object_class, object_name): - msg = f"Object = `{object_name}` does not exist for class `{object_class}`." - raise NotFoundError(msg) - - # Set defaults - collection = collection or get_default_collection(object_class) - parent_class = parent_class or ClassEnum.System - - # Validate property exists for this collection - valid_properties = self.list_valid_properties( - collection, child_class_enum=object_class, parent_class_enum=parent_class - ) - if property_name not in valid_properties: - msg = ( - f"Property '{property_name}' does not exist for collection: {collection}. " - f"Run `self.list_valid_properties({collection})` to verify valid properties." - ) - raise NameError(msg) - - # Get IDs for the property lookup - property_id = self.get_property_id( - property_name, - parent_class_enum=parent_class, - child_class_enum=object_class, - collection_enum=collection, - ) - - membership_id = resolve_membership_id( - self, + collection, parent_class, property_id, membership_id = self._resolve_property_context( object_name, + property_name, object_class=object_class, collection=collection, parent_class=parent_class, @@ -4570,32 +4555,13 @@ def _update_property_value( parent_class: ClassEnum | None, ) -> None: """Update matching property rows inside an active transaction.""" - if not checks_module.check_object_exists(self, object_class, object_name): - raise NotFoundError(f"Object = `{object_name}` does not exist for class `{object_class}`.") - - collection = collection or get_default_collection(object_class) - parent_class = parent_class or ClassEnum.System - valid_properties = self.list_valid_properties( - collection, - parent_class_enum=parent_class, - child_class_enum=object_class, - ) - if property_name not in valid_properties: - raise NameError(f"Property {property_name} does not exist for collection: {collection}.") - - membership_id = resolve_membership_id( - self, + collection, parent_class, property_id, membership_id = self._resolve_property_context( object_name, + property_name, object_class=object_class, collection=collection, parent_class=parent_class, ) - property_id = self.get_property_id( - property_name, - collection_enum=collection, - child_class_enum=object_class, - parent_class_enum=parent_class, - ) conditions = ["d.membership_id = ?", "d.property_id = ?"] params: list[Any] = [membership_id, property_id] diff --git a/tests/test_plexosdb_properties.py b/tests/test_plexosdb_properties.py index d50c588..5ce60b2 100644 --- a/tests/test_plexosdb_properties.py +++ b/tests/test_plexosdb_properties.py @@ -1,6 +1,28 @@ import pytest -from plexosdb import ClassEnum +from plexosdb import ClassEnum, CollectionEnum + + +def test_resolve_property_context_uses_defaults_and_shared_ids(db_with_topology): + collection, parent_class, property_id, membership_id = db_with_topology._resolve_property_context( + "thermal-01", + "Max Capacity", + object_class=ClassEnum.Generator, + ) + + assert collection == CollectionEnum.Generators + assert parent_class == ClassEnum.System + assert property_id == db_with_topology.get_property_id( + "Max Capacity", + collection_enum=CollectionEnum.Generators, + child_class_enum=ClassEnum.Generator, + parent_class_enum=ClassEnum.System, + ) + assert membership_id == db_with_topology.get_membership_id( + "System", + "thermal-01", + CollectionEnum.Generators, + ) def test_add_property_to_object_succeeds(db_with_topology): From 75dea8dd17dd2fec0d204211e0d4bef27e316f9a Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 12:06:54 -0600 Subject: [PATCH 07/10] fix: reuse function on context correctly --- src/plexosdb/db.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plexosdb/db.py b/src/plexosdb/db.py index a6603d6..4d4a883 100644 --- a/src/plexosdb/db.py +++ b/src/plexosdb/db.py @@ -1193,13 +1193,12 @@ def _resolve_property_context( collection = collection or get_default_collection(object_class) parent_class = parent_class or ClassEnum.System - valid_properties = self.list_valid_properties( + property_name = self._validate_properties( + property_name, collection, - parent_class_enum=parent_class, - child_class_enum=object_class, - ) - if property_name not in valid_properties: - raise NameError(f"Property '{property_name}' does not exist for collection: {collection}.") + object_class, + parent_class=parent_class, + )[0] property_id = self.get_property_id( property_name, @@ -4671,8 +4670,11 @@ def _validate_properties( props, parent_class=parent_class, ): + property_detail = ( + f"Property '{props[0]}' does not exist" if len(props) == 1 else f"Invalid properties {props}" + ) msg = ( - f"Invalid property {props} for collection={collection}. " + f"{property_detail} for collection: {collection}. " "Use `list_valid_properties()` to check valid properties." ) raise NameError(msg) From 0d934972db4fafd3753b2c97eb03ac29b51d4916 Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 12:10:21 -0600 Subject: [PATCH 08/10] fix: preserve explicit parent membership selection --- src/plexosdb/db.py | 40 ++++++++++++++++++---- tests/test_plexosdb_update_property.py | 46 +++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/src/plexosdb/db.py b/src/plexosdb/db.py index 4d4a883..55306fc 100644 --- a/src/plexosdb/db.py +++ b/src/plexosdb/db.py @@ -4390,12 +4390,17 @@ def _prepare_property_update_group( (collection_id,), ) property_ids = {name: property_id for name, property_id in property_rows} - membership_map = _resolve_membership_map( - self, - [{"name": update["object_name"]} for update in group], - object_class=object_class, - parent_class=parent_class, - collection=collection, + implicit_parent_updates = [update for update in group if update.get("parent_object_name") is None] + membership_map = ( + _resolve_membership_map( + self, + [{"name": update["object_name"]} for update in implicit_parent_updates], + object_class=object_class, + parent_class=parent_class, + collection=collection, + ) + if implicit_parent_updates + else {} ) scenario_ids = self._resolve_scenario_ids(group, scenario_class_id) selectors = self._prepare_property_update_selectors( @@ -4405,6 +4410,8 @@ def _prepare_property_update_group( membership_map=membership_map, scenario_ids=scenario_ids, collection=collection, + object_class=object_class, + parent_class=parent_class, ) data_ids_by_index = self._find_property_data_ids(selectors, scenario_class_id) @@ -4443,6 +4450,8 @@ def _prepare_property_update_selectors( membership_map: dict[str, int], scenario_ids: dict[str, int], collection: CollectionEnum, + object_class: ClassEnum, + parent_class: ClassEnum, ) -> list[tuple[int, int, int, int | None, int | None]]: selectors: list[tuple[int, int, int, int | None, int | None]] = [] for index, update in enumerate(group): @@ -4458,10 +4467,23 @@ def _prepare_property_update_selectors( band_id = int(band) if band is not None else None except (TypeError, ValueError) as exc: raise ValueError(f"Band must be an integer, got {band!r}.") from exc + parent_object_name = update.get("parent_object_name") + membership_id = ( + resolve_membership_id( + self, + update["object_name"], + object_class=object_class, + collection=collection, + parent_class=parent_class, + parent_object_name=parent_object_name, + ) + if parent_object_name is not None + else membership_map[update["object_name"]] + ) selectors.append( ( index, - membership_map[update["object_name"]], + membership_id, property_ids[property_name], scenario_id, band_id, @@ -4523,6 +4545,7 @@ def update_property( band: str | None = None, collection: CollectionEnum | None = None, parent_class: ClassEnum | None = None, + parent_object_name: str | None = None, ) -> None: """Update a property value for a given object.""" self.update_properties( @@ -4536,6 +4559,7 @@ def update_property( "band": band, "collection": collection, "parent_class": parent_class, + "parent_object_name": parent_object_name, } ] ) @@ -4552,6 +4576,7 @@ def _update_property_value( band: str | int | None, collection: CollectionEnum | None, parent_class: ClassEnum | None, + parent_object_name: str | None, ) -> None: """Update matching property rows inside an active transaction.""" collection, parent_class, property_id, membership_id = self._resolve_property_context( @@ -4560,6 +4585,7 @@ def _update_property_value( object_class=object_class, collection=collection, parent_class=parent_class, + parent_object_name=parent_object_name, ) conditions = ["d.membership_id = ?", "d.property_id = ?"] diff --git a/tests/test_plexosdb_update_property.py b/tests/test_plexosdb_update_property.py index 8778db4..651f103 100644 --- a/tests/test_plexosdb_update_property.py +++ b/tests/test_plexosdb_update_property.py @@ -2,7 +2,7 @@ import pytest -from plexosdb import ClassEnum, PlexosDB +from plexosdb import ClassEnum, CollectionEnum, PlexosDB from plexosdb.exceptions import NameError, NotFoundError @@ -113,6 +113,50 @@ def test_update_properties_rolls_back_domain_validation_failure_and_reuses_conne assert _property_rows(run_of_river_db, "Coal_Gen", "Max Capacity") == [(625.0, 1)] +def test_update_property_uses_explicit_parent_membership(db_with_reserve_collection_property) -> None: + db = db_with_reserve_collection_property + db.add_object(ClassEnum.Reserve, "TestReserve2") + db.add_membership( + parent_class_enum=ClassEnum.Reserve, + child_class_enum=ClassEnum.Region, + parent_object_name="TestReserve2", + child_object_name="region-01", + collection_enum=CollectionEnum.Regions, + ) + db.add_property( + ClassEnum.Region, + "region-01", + "Load Risk", + 7.0, + collection_enum=CollectionEnum.Regions, + parent_class_enum=ClassEnum.Reserve, + parent_object_name="TestReserve2", + ) + + db.update_property( + "region-01", + "Load Risk", + 8.0, + object_class=ClassEnum.Region, + collection=CollectionEnum.Regions, + parent_class=ClassEnum.Reserve, + parent_object_name="TestReserve2", + ) + + rows = db.query( + """ + SELECT parent_object.name, d.value + FROM t_data AS d + JOIN t_membership AS m ON m.membership_id = d.membership_id + JOIN t_object AS parent_object ON parent_object.object_id = m.parent_object_id + JOIN t_property AS p ON p.property_id = d.property_id + WHERE p.name = 'Load Risk' + ORDER BY parent_object.name + """ + ) + assert rows == [("TestReserve", 6.0), ("TestReserve2", 8.0)] + + def test_update_property_raises_for_missing_fixture_property(run_of_river_db: PlexosDB) -> None: with pytest.raises(NotFoundError): run_of_river_db.update_property( From 8a05c3e1f80f463d9d74ec93dfee008e57544896 Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 12:13:29 -0600 Subject: [PATCH 09/10] fix: use existing public property readers in test handling --- tests/test_plexosdb_update_property.py | 33 +++++++------------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/tests/test_plexosdb_update_property.py b/tests/test_plexosdb_update_property.py index 651f103..8a11fb1 100644 --- a/tests/test_plexosdb_update_property.py +++ b/tests/test_plexosdb_update_property.py @@ -17,19 +17,8 @@ def run_of_river_db(): def _property_rows(db: PlexosDB, object_name: str, property_name: str) -> list[tuple]: - return db.query( - """ - SELECT d.value, COALESCE(b.band_id, 1) - FROM t_data AS d - JOIN t_membership AS m ON m.membership_id = d.membership_id - JOIN t_object AS o ON o.object_id = m.child_object_id - JOIN t_property AS p ON p.property_id = d.property_id - LEFT JOIN t_band AS b ON b.data_id = d.data_id - WHERE o.name = ? AND p.name = ? - ORDER BY COALESCE(b.band_id, 1) - """, - (object_name, property_name), - ) + properties = db.get_object_properties(ClassEnum.Generator, object_name, property_names=property_name) + return sorted((property["value"], property.get("band") or 1) for property in properties) def test_update_property_updates_xml_fixture_value(run_of_river_db: PlexosDB) -> None: @@ -143,18 +132,14 @@ def test_update_property_uses_explicit_parent_membership(db_with_reserve_collect parent_object_name="TestReserve2", ) - rows = db.query( - """ - SELECT parent_object.name, d.value - FROM t_data AS d - JOIN t_membership AS m ON m.membership_id = d.membership_id - JOIN t_object AS parent_object ON parent_object.object_id = m.parent_object_id - JOIN t_property AS p ON p.property_id = d.property_id - WHERE p.name = 'Load Risk' - ORDER BY parent_object.name - """ + rows = db.get_object_properties( + ClassEnum.Region, + "region-01", + property_names="Load Risk", + parent_class_enum=ClassEnum.Reserve, + collection_enum=CollectionEnum.Regions, ) - assert rows == [("TestReserve", 6.0), ("TestReserve2", 8.0)] + assert sorted(property["value"] for property in rows) == [6.0, 8.0] def test_update_property_raises_for_missing_fixture_property(run_of_river_db: PlexosDB) -> None: From 11cea1f86c8de7a8ad2a4a71d3c534e5703340ee Mon Sep 17 00:00:00 2001 From: mvelasqu Date: Thu, 13 Aug 2026 13:03:57 -0600 Subject: [PATCH 10/10] docs: write missing docstring on files --- src/plexosdb/db.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/plexosdb/db.py b/src/plexosdb/db.py index 55306fc..432cf7d 100644 --- a/src/plexosdb/db.py +++ b/src/plexosdb/db.py @@ -4328,6 +4328,7 @@ def update_properties(self, updates: list[dict[str, Any]]) -> None: self._db.executemany("UPDATE t_data SET value = ? WHERE data_id = ?", prepared_updates) def _prepare_property_updates(self, updates: list[dict[str, Any]]) -> list[tuple[Any, int]]: + """Resolve bulk property updates into value/data-ID parameter pairs.""" grouped_updates: dict[tuple[ClassEnum, CollectionEnum, ClassEnum], list[dict[str, Any]]] = {} for update in updates: object_class = update["object_class"] @@ -4361,6 +4362,7 @@ def _prepare_property_update_group( parent_class: ClassEnum, scenario_class_id: int, ) -> list[tuple[Any, int]]: + """Validate one update group and resolve its matching data rows.""" object_names = tuple({update["object_name"] for update in group}) object_class_id = self.get_class_id(object_class) object_placeholders = ", ".join("?" for _ in object_names) @@ -4431,6 +4433,7 @@ def _prepare_property_update_group( return prepared_updates def _resolve_scenario_ids(self, group: list[dict[str, Any]], scenario_class_id: int) -> dict[str, int]: + """Resolve scenario names in an update group to object IDs.""" scenario_names = tuple({update["scenario"] for update in group if update.get("scenario") is not None}) if not scenario_names: return {} @@ -4453,6 +4456,7 @@ def _prepare_property_update_selectors( object_class: ClassEnum, parent_class: ClassEnum, ) -> list[tuple[int, int, int, int | None, int | None]]: + """Validate update selectors and resolve their membership and property IDs.""" selectors: list[tuple[int, int, int, int | None, int | None]] = [] for index, update in enumerate(group): property_name = update["property_name"] @@ -4496,6 +4500,7 @@ def _find_property_data_ids( selectors: list[tuple[int, int, int, int | None, int | None]], scenario_class_id: int, ) -> dict[int, list[int]]: + """Find data IDs matching selectors in bounded SQL batches.""" data_ids_by_index: dict[int, list[int]] = {} for selector_batch in batched(selectors, 180): values_sql = ", ".join("(?, ?, ?, ?, ?)" for _ in selector_batch)