From 0dbe9cda83382733fc1577390652d402a00c12ab Mon Sep 17 00:00:00 2001 From: Chris Mutel Date: Tue, 19 Nov 2024 11:40:30 +0100 Subject: [PATCH 1/4] Turn ORM calls into event-capturable ones * `.delete()` isn't ran by the ORM layer but rather as a direct query and as such the approach with signals on `.save()` doesn't work. However, `.delete_instance()` is an ORM-layer call. This is at the expense of performance as all the entities to be deleted need to be looped over. See https://docs.peewee-orm.com/en/latest/peewee/playhouse.html#signals for more details. --- bw2data/parameters.py | 48 ++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/bw2data/parameters.py b/bw2data/parameters.py index e9276145..44a4fa36 100644 --- a/bw2data/parameters.py +++ b/bw2data/parameters.py @@ -1187,10 +1187,12 @@ def add_to_group(self, group, activity): return # Avoid duplicate by deleting existing parameters - ActivityParameter.delete().where( + # Call in loop to get event handling + for ap in ActivityParameter.select().where( ActivityParameter.database == activity["database"], ActivityParameter.code == activity["code"], - ).execute() + ): + ap.delete_instance() def reformat(o): skipped = ("name", "amount", "formula") @@ -1258,10 +1260,12 @@ def drop_fields(dct): with self.db.atomic(): self.remove_exchanges_from_group(group, activity, restore_amounts) - ActivityParameter.delete().where( + # Call in loop to get event handling + for ap in ActivityParameter.select().where( ActivityParameter.database == activity[0], ActivityParameter.code == activity[1], - ).execute() + ): + ap.delete_instance() activity.save() def add_exchanges_to_group(self, group, activity): @@ -1315,10 +1319,12 @@ def remove_exchanges_from_group(self, group, activity, restore_original=True): del exc["original_amount"] exc.save() - ParameterizedExchange.delete().where(ParameterizedExchange.group == group).execute() + # Call in loop to get event handling + for pe in ParameterizedExchange.select().where(ParameterizedExchange.group == group): + pe.delete_instance() def new_project_parameters(self, data, overwrite=True): - """Efficiently and correctly enter multiple parameters. + """Correctly enter multiple parameters. Will overwrite existing project parameters with the same name, unless ``overwrite`` is false, in which case a ``ValueError`` is raised. @@ -1361,14 +1367,16 @@ def reformat(ds): with self.db.atomic(): # Remove existing values - ProjectParameter.delete().where(ProjectParameter.name << tuple(new)).execute() - for idx in range(0, len(data), 100): - ProjectParameter.insert_many(data[idx : idx + 100]).execute() + # Call in loop to get event handling + for pp in ProjectParameter.select().where(ProjectParameter.name << tuple(new)): + pp.delete_instance() + for dataset in data: + ProjectParameter.create(**dataset) Group.get_or_create(name="project")[0].expire() ProjectParameter.recalculate() def new_database_parameters(self, data, database, overwrite=True): - """Efficiently and correctly enter multiple parameters. Deletes **all** existing database parameters for this database. + """Correctly enter multiple parameters. Deletes **all** existing database parameters for this database. Will overwrite existing database parameters with the same name, unless ``overwrite`` is false, in which case a ``ValueError`` is raised. @@ -1419,17 +1427,18 @@ def reformat(ds): with self.db.atomic(): # Remove existing values - DatabaseParameter.delete().where( + for dp in DatabaseParameter.select().where( DatabaseParameter.database == database, DatabaseParameter.name << tuple(new), - ).execute() - for idx in range(0, len(data), 100): - DatabaseParameter.insert_many(data[idx : idx + 100]).execute() + ): + dp.delete_instance() + for dataset in data: + DatabaseParameter.create(**dataset) Group.get_or_create(name=database)[0].expire() DatabaseParameter.recalculate(database) def new_activity_parameters(self, data, group, overwrite=True): - """Efficiently and correctly enter multiple parameters. Deletes **all** existing activity parameters for this group. + """Correctly enter multiple parameters. Deletes **all** existing activity parameters for this group. Will overwrite existing parameters in the same group with the same name, unless ``overwrite`` is false, in which case a ``ValueError`` is raised. @@ -1490,11 +1499,12 @@ def reformat(ds): with self.db.atomic(): # Remove existing values - ActivityParameter.delete().where( + for ap in ActivityParameter.select().where( ActivityParameter.group == group, ActivityParameter.name << new - ).execute() - for idx in range(0, len(data), 100): - ActivityParameter.insert_many(data[idx : idx + 100]).execute() + ): + ap.delete_instance() + for dataset in data: + ActivityParameter.create(**dataset) Group.get_or_create(name=group)[0].expire() ActivityParameter.recalculate(group) From ffb70a0241889a2f92b80215ffd68101e2157ec5 Mon Sep 17 00:00:00 2001 From: Chris Mutel Date: Tue, 19 Nov 2024 12:00:48 +0100 Subject: [PATCH 2/4] Add events to `Group` --- bw2data/parameters.py | 2 +- bw2data/revisions.py | 9 ++ tests/unit/test_group.py | 252 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_group.py diff --git a/bw2data/parameters.py b/bw2data/parameters.py index 44a4fa36..80a4f69d 100644 --- a/bw2data/parameters.py +++ b/bw2data/parameters.py @@ -1106,7 +1106,7 @@ def recalculate(group): return ActivityParameter.recalculate_exchanges(group) -class Group(Model): +class Group(SnowflakeIDBaseClass): name = TextField(unique=True) fresh = BooleanField(default=True) updated = DateTimeField(default=datetime.datetime.now) diff --git a/bw2data/revisions.py b/bw2data/revisions.py index 15a6dbac..3e9bb89d 100644 --- a/bw2data/revisions.py +++ b/bw2data/revisions.py @@ -12,6 +12,7 @@ from bw2data.parameters import ( ActivityParameter, DatabaseParameter, + Group, ParameterBase, ParameterizedExchange, ProjectParameter, @@ -322,6 +323,12 @@ def _unwrap_diff_dict(cls, data: dict) -> dict: } +class RevisionedGroup(RevisionedParameter): + KEYS = ("id", "name", "order") + ORM_CLASS = Group + # Implicitly skips `fresh` and `updated` fields because they are in `KEYS`. + + class RevisionedParameterizedExchange(RevisionedParameter): KEYS = ("id", "group", "formula", "exchange") ORM_CLASS = ParameterizedExchange @@ -465,6 +472,7 @@ def handle(cls, revision_data: dict) -> None: DatabaseParameter: "database_parameter", ActivityParameter: "activity_parameter", ParameterizedExchange: "parameterized_exchange", + Group: "group", } REVISIONED_LABEL_AS_OBJECT = { "lci_node": RevisionedNode, @@ -474,5 +482,6 @@ def handle(cls, revision_data: dict) -> None: "database_parameter": RevisionedDatabaseParameter, "activity_parameter": RevisionedActivityParameter, "parameterized_exchange": RevisionedParameterizedExchange, + "group": RevisionedGroup, } REVISIONS_OBJECT_AS_LABEL = {v: k for k, v in REVISIONED_LABEL_AS_OBJECT.items()} diff --git a/tests/unit/test_group.py b/tests/unit/test_group.py new file mode 100644 index 00000000..4f0d8427 --- /dev/null +++ b/tests/unit/test_group.py @@ -0,0 +1,252 @@ +import json + +from bw2data.parameters import Group +from bw2data.project import projects +from bw2data.snowflake_ids import snowflake_id_generator +from bw2data.tests import bw2test + + +@bw2test +def test_group_revision_expected_format_create(num_revisions): + projects.set_current("activity-event") + + projects.dataset.set_sourced() + assert projects.dataset.revision is None + + group = Group.create(name="A", order=[]) + + assert projects.dataset.revision is not None + with open(projects.dataset.dir / "revisions" / f"{projects.dataset.revision}.rev", "r") as f: + revision = json.load(f) + + expected = { + "metadata": { + "parent_revision": None, + "revision": projects.dataset.revision, + "authors": "Anonymous", + "title": "Untitled revision", + "description": "No description", + }, + "data": [ + { + "type": "group", + "id": group.id, + "change_type": "create", + "delta": { + "type_changes": { + "root": { + "old_type": "NoneType", + "new_type": "dict", + "new_value": {"id": group.id, "name": "A", "order": []}, + } + } + }, + } + ], + } + + assert revision == expected + assert num_revisions(projects) == 1 + + +@bw2test +def test_group_revision_apply_create(num_revisions): + projects.set_current("activity-event") + + projects.dataset.set_sourced() + assert projects.dataset.revision is None + + revision_id = next(snowflake_id_generator) + group_id = next(snowflake_id_generator) + revision = { + "metadata": { + "parent_revision": None, + "revision": revision_id, + "authors": "Anonymous", + "title": "Untitled revision", + "description": "No description", + }, + "data": [ + { + "type": "group", + "id": group_id, + "change_type": "create", + "delta": { + "type_changes": { + "root": { + "old_type": "NoneType", + "new_type": "dict", + "new_value": {"id": group_id, "name": "A", "order": []}, + } + } + }, + } + ], + } + + projects.dataset.apply_revision(revision) + assert projects.dataset.revision == revision_id + assert not num_revisions(projects) + + group = Group.get(Group.id == group_id) + assert group.name == "A" + assert group.order == [] + + +@bw2test +def test_group_revision_expected_format_delete(num_revisions): + projects.set_current("activity-event") + + group = Group.create(name="A", order=[]) + + projects.dataset.set_sourced() + assert projects.dataset.revision is None + + group.delete_instance() + + with open(projects.dataset.dir / "revisions" / f"{projects.dataset.revision}.rev", "r") as f: + revision = json.load(f) + + expected = { + "metadata": { + "parent_revision": None, + "revision": projects.dataset.revision, + "authors": "Anonymous", + "title": "Untitled revision", + "description": "No description", + }, + "data": [ + { + "type": "group", + "id": group.id, + "change_type": "delete", + "delta": { + "type_changes": { + "root": {"old_type": "dict", "new_type": "NoneType", "new_value": None} + } + }, + } + ], + } + + assert revision == expected + assert num_revisions(projects) == 1 + + +@bw2test +def test_group_revision_apply_delete(num_revisions): + projects.set_current("activity-event") + + group = Group.create(name="A", order=[]) + + projects.dataset.set_sourced() + assert projects.dataset.revision is None + + revision_id = next(snowflake_id_generator) + + revision = { + "metadata": { + "parent_revision": None, + "revision": revision_id, + "authors": "Anonymous", + "title": "Untitled revision", + "description": "No description", + }, + "data": [ + { + "type": "group", + "id": group.id, + "change_type": "delete", + "delta": { + "type_changes": { + "root": {"old_type": "dict", "new_type": "NoneType", "new_value": None} + } + }, + } + ], + } + + projects.dataset.apply_revision(revision) + assert projects.dataset.revision == revision_id + assert not Group.select().count() + assert not num_revisions(projects) + + +@bw2test +def test_group_revision_expected_format_update(num_revisions): + projects.set_current("activity-event") + + group = Group.create(name="A", order=[]) + + projects.dataset.set_sourced() + assert projects.dataset.revision is None + + group.order = ["foo", "bar"] + group.save() + + parent = projects.dataset.revision + assert parent is not None + + with open(projects.dataset.dir / "revisions" / f"{projects.dataset.revision}.rev", "r") as f: + revision = json.load(f) + + expected = { + "metadata": { + "parent_revision": None, + "revision": projects.dataset.revision, + "authors": "Anonymous", + "title": "Untitled revision", + "description": "No description", + }, + "data": [ + { + "type": "group", + "id": group.id, + "change_type": "update", + "delta": { + "iterable_item_added": {"root['order'][0]": "foo", "root['order'][1]": "bar"} + }, + } + ], + } + + assert revision == expected + assert num_revisions(projects) == 1 + + +@bw2test +def test_group_revision_apply_update(num_revisions): + projects.set_current("activity-event") + + group = Group.create(name="A", order=[]) + + projects.dataset.set_sourced() + assert projects.dataset.revision is None + + revision_id = next(snowflake_id_generator) + + revision = { + "metadata": { + "parent_revision": None, + "revision": revision_id, + "authors": "Anonymous", + "title": "Untitled revision", + "description": "No description", + }, + "data": [ + { + "type": "group", + "id": group.id, + "change_type": "update", + "delta": { + "iterable_item_added": {"root['order'][0]": "foo", "root['order'][1]": "bar"} + }, + } + ], + } + + projects.dataset.apply_revision(revision) + assert projects.dataset.revision == revision_id + group = Group.get(Group.id == group.id) + assert group.order == ["foo", "bar"] + assert not num_revisions(projects) From 8119a90b44333f86d26d0bb908b697b414a92fc1 Mon Sep 17 00:00:00 2001 From: Chris Mutel Date: Wed, 20 Nov 2024 20:21:37 +0100 Subject: [PATCH 3/4] Don't generate diff if nothing changed --- bw2data/revisions.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/bw2data/revisions.py b/bw2data/revisions.py index 3e9bb89d..235a8d3f 100644 --- a/bw2data/revisions.py +++ b/bw2data/revisions.py @@ -186,15 +186,19 @@ def generate( label = SIGNALLEDOBJECT_TO_LABEL[obj_type] handler = REVISIONED_LABEL_AS_OBJECT[label] + diff = deepdiff.DeepDiff( + handler.current_state_as_dict(old) if old else None, + handler.current_state_as_dict(new) if new else None, + verbose_level=2, + ) + if not diff: + return None + return cls.from_difference( label, old.id if old is not None else new.id, change_type, - deepdiff.DeepDiff( - handler.current_state_as_dict(old) if old else None, - handler.current_state_as_dict(new) if new else None, - verbose_level=2, - ), + diff, ) From 547072913106811227b66daa93b5fb53f1bb82c3 Mon Sep 17 00:00:00 2001 From: Chris Mutel Date: Wed, 20 Nov 2024 20:35:23 +0100 Subject: [PATCH 4/4] Monkeypatch to stop group events showing up in other tests --- tests/unit/test_activity_parameter_events.py | 27 ++++++++++++++++-- tests/unit/test_database_parameter_events.py | 18 ++++++++++-- tests/unit/test_project_parameter_events.py | 29 ++++++++++++++++---- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_activity_parameter_events.py b/tests/unit/test_activity_parameter_events.py index b5c54d97..e6033645 100644 --- a/tests/unit/test_activity_parameter_events.py +++ b/tests/unit/test_activity_parameter_events.py @@ -8,7 +8,13 @@ @bw2test -def test_activity_parameter_revision_expected_format_create(num_revisions): +def test_activity_parameter_revision_expected_format_create(num_revisions, monkeypatch): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") assert not ActivityParameter.select().count() @@ -27,6 +33,17 @@ def test_activity_parameter_revision_expected_format_create(num_revisions): amount=5, data={"foo": "bar"}, ) + + from pprint import pprint + + pprint( + [ + json.load(open(fp)) + for fp in (projects.dataset.dir / "revisions").iterdir() + if fp.stem.lower() != "head" and fp.is_file() + ] + ) + assert dp.id > 1e6 assert num_revisions(projects) == 1 @@ -73,7 +90,13 @@ def test_activity_parameter_revision_expected_format_create(num_revisions): @bw2test -def test_activity_parameter_revision_apply_create(num_revisions): +def test_activity_parameter_revision_apply_create(num_revisions, monkeypatch): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") DatabaseChooser("test-database").register() assert projects.dataset.revision is None diff --git a/tests/unit/test_database_parameter_events.py b/tests/unit/test_database_parameter_events.py index ac80063d..223d01a1 100644 --- a/tests/unit/test_database_parameter_events.py +++ b/tests/unit/test_database_parameter_events.py @@ -1,14 +1,20 @@ import json from bw2data.database import DatabaseChooser -from bw2data.parameters import DatabaseParameter +from bw2data.parameters import DatabaseParameter, Group from bw2data.project import projects from bw2data.snowflake_ids import snowflake_id_generator from bw2data.tests import bw2test @bw2test -def test_database_parameter_revision_expected_format_create(num_revisions): +def test_database_parameter_revision_expected_format_create(num_revisions, monkeypatch): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") assert not DatabaseParameter.select().count() @@ -65,7 +71,13 @@ def test_database_parameter_revision_expected_format_create(num_revisions): @bw2test -def test_database_parameter_revision_apply_create(num_revisions): +def test_database_parameter_revision_apply_create(num_revisions, monkeypatch): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") DatabaseChooser("test-example").register() assert projects.dataset.revision is None diff --git a/tests/unit/test_project_parameter_events.py b/tests/unit/test_project_parameter_events.py index dd90bf5f..9c944c3f 100644 --- a/tests/unit/test_project_parameter_events.py +++ b/tests/unit/test_project_parameter_events.py @@ -1,14 +1,19 @@ import json -from bw2data.database import DatabaseChooser -from bw2data.parameters import ProjectParameter +from bw2data.parameters import Group, ProjectParameter from bw2data.project import projects from bw2data.snowflake_ids import snowflake_id_generator from bw2data.tests import bw2test @bw2test -def test_project_parameter_revision_expected_format_create(num_revisions): +def test_project_parameter_revision_expected_format_create(num_revisions, monkeypatch): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") assert not ProjectParameter.select().count() @@ -59,7 +64,13 @@ def test_project_parameter_revision_expected_format_create(num_revisions): @bw2test -def test_project_parameter_revision_apply_create(num_revisions): +def test_project_parameter_revision_apply_create(num_revisions, monkeypatch): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") projects.dataset.set_sourced() assert projects.dataset.revision is None @@ -355,7 +366,15 @@ def fake_recalculate(ignored=None, signal=True): @bw2test -def test_project_parameter_revision_expected_format_update_formula_parameter_name(num_revisions): +def test_project_parameter_revision_expected_format_update_formula_parameter_name( + num_revisions, monkeypatch +): + def no_signal_save(self, *args, **kwargs): + kwargs["signal"] = False + return super(Group, self).save(*args, **kwargs) + + monkeypatch.setattr(Group, "save", no_signal_save) + projects.set_current("activity-event") assert projects.dataset.revision is None