From b88fc90e7fab898a19f6ba3c4a0a1543847215f9 Mon Sep 17 00:00:00 2001 From: dlbrittain Date: Fri, 10 May 2024 12:59:18 -0700 Subject: [PATCH 1/3] fix: remove inline update trigger --- dynamicannotationdb/annotation.py | 59 +------------------------------ 1 file changed, 1 insertion(+), 58 deletions(-) diff --git a/dynamicannotationdb/annotation.py b/dynamicannotationdb/annotation.py index 3b1421f..fd291fc 100644 --- a/dynamicannotationdb/annotation.py +++ b/dynamicannotationdb/annotation.py @@ -96,12 +96,11 @@ def create_table( existing_tables = self.db._check_table_is_unique(table_name) if table_metadata: - reference_table, track_updates = self.schema._parse_schema_metadata_params( + reference_table, _ = self.schema._parse_schema_metadata_params( schema_type, table_name, table_metadata, existing_tables ) else: reference_table = None - track_updates = None AnnotationModel = self.schema.create_annotation_model( table_name, @@ -109,20 +108,6 @@ def create_table( table_metadata=table_metadata, with_crud_columns=with_crud_columns, ) - if hasattr(AnnotationModel, "target_id") and reference_table: - - reference_table_name = self.db.get_table_sql_metadata(reference_table) - logging.info( - f"{table_name} is targeting reference table: {reference_table_name}" - ) - if track_updates: - self.create_reference_update_trigger( - table_name, reference_table, AnnotationModel - ) - description += ( - f" [Note: This table '{AnnotationModel.__name__}' will update the 'target_id' " - f"foreign_key when updates are made to the '{reference_table}' table] " - ) self.db.base.metadata.tables[AnnotationModel.__name__].create( bind=self.db.engine @@ -229,48 +214,6 @@ def update_table_metadata( logging.info(f"Table: {table_name} metadata updated ") return self.db.get_table_metadata(table_name) - def create_reference_update_trigger(self, table_name, reference_table, model): - func_name = f"{table_name}_update_reference_id" - func = DDL( - f""" - CREATE or REPLACE function {func_name}() - returns TRIGGER - as $func$ - begin - if EXISTS - (SELECT 1 - FROM information_schema.columns - WHERE table_name='{reference_table}' - AND column_name='superceded_id') THEN - update {table_name} ref - set target_id = new.superceded_id - where ref.target_id = old.id; - return new; - else - return NULL; - END if; - end; - $func$ language plpgsql; - """ - ) - trigger = DDL( - f"""CREATE TRIGGER update_{table_name}_target_id AFTER UPDATE ON {reference_table} - FOR EACH ROW EXECUTE PROCEDURE {func_name}();""" - ) - - event.listen( - model.__table__, - "after_create", - func.execute_if(dialect="postgresql"), - ) - - event.listen( - model.__table__, - "after_create", - trigger.execute_if(dialect="postgresql"), - ) - return True - def delete_table(self, table_name: str) -> bool: """Marks a table for deletion, which will remove it from user visible calls From d1b6f305a95dcdd6ff070f328f44931d668617e2 Mon Sep 17 00:00:00 2001 From: dlbrittain Date: Fri, 10 May 2024 13:02:49 -0700 Subject: [PATCH 2/3] feat: allow partial row info for updating --- dynamicannotationdb/annotation.py | 56 ++++++++++++++++--------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/dynamicannotationdb/annotation.py b/dynamicannotationdb/annotation.py index fd291fc..8f451e1 100644 --- a/dynamicannotationdb/annotation.py +++ b/dynamicannotationdb/annotation.py @@ -353,7 +353,8 @@ def update_annotation(self, table_name: str, annotation: dict) -> str: table_name : str name of targeted table to update annotations annotation : dict - new data for that annotation + new data for that annotation, allows for partial updates but + requires an 'id' field to target the row Returns ------- @@ -370,8 +371,27 @@ def update_annotation(self, table_name: str, annotation: dict) -> str: return "Annotation requires an 'id' to update targeted row" schema_type, AnnotationModel = self._load_model(table_name) + try: + old_anno = ( + self.db.cached_session.query(AnnotationModel) + .filter(AnnotationModel.id == anno_id) + .one() + ) + except NoAnnotationsFoundWithID as e: + raise f"No result found for {anno_id}. Error: {e}" from e + + if old_anno.superceded_id: + raise UpdateAnnotationError(anno_id, old_anno.superceded_id) + + # Merge old data with new changes + old_data = { + column.name: getattr(old_anno, column.name) + for column in old_anno.__table__.columns + } + updated_data = {**old_data, **annotation} + new_annotation, __ = self.schema.split_flattened_schema_data( - schema_type, annotation + schema_type, updated_data ) if hasattr(AnnotationModel, "created"): @@ -381,32 +401,14 @@ def update_annotation(self, table_name: str, annotation: dict) -> str: new_data = AnnotationModel(**new_annotation) - try: - old_anno = ( - self.db.cached_session.query(AnnotationModel) - .filter(AnnotationModel.id == anno_id) - .one() - ) - except NoAnnotationsFoundWithID as e: - raise f"No result found for {anno_id}. Error: {e}" from e - if hasattr(AnnotationModel, "target_id"): - new_data_map = self.db.get_automap_items(new_data) - for column_name, value in new_data_map.items(): - setattr(old_anno, column_name, value) - old_anno.valid = True - update_map = {anno_id: old_anno.id} - else: - if old_anno.superceded_id: - raise UpdateAnnotationError(anno_id, old_anno.superceded_id) - - self.db.cached_session.add(new_data) - self.db.cached_session.flush() + self.db.cached_session.add(new_data) + self.db.cached_session.flush() - deleted_time = datetime.datetime.utcnow() - old_anno.deleted = deleted_time - old_anno.superceded_id = new_data.id - old_anno.valid = False - update_map = {anno_id: new_data.id} + deleted_time = datetime.datetime.utcnow() + old_anno.deleted = deleted_time + old_anno.superceded_id = new_data.id + old_anno.valid = False + update_map = {anno_id: new_data.id} ( self.db.cached_session.query(AnnoMetadata) From be8f30b197740c5788aba015616b3cf9bb2a7750 Mon Sep 17 00:00:00 2001 From: dlbrittain Date: Fri, 10 May 2024 13:03:27 -0700 Subject: [PATCH 3/3] test: update tests --- tests/test_annotation.py | 97 +++++++++++++++++++++++++++++++++++----- tests/test_database.py | 2 +- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/tests/test_annotation.py b/tests/test_annotation.py index 9195132..348aada 100644 --- a/tests/test_annotation.py +++ b/tests/test_annotation.py @@ -26,7 +26,6 @@ def test_create_table(dadb_interface, annotation_metadata): def test_create_all_schema_types(dadb_interface, annotation_metadata): - vx = annotation_metadata["voxel_resolution_x"] vy = annotation_metadata["voxel_resolution_y"] vz = annotation_metadata["voxel_resolution_z"] @@ -75,7 +74,7 @@ def test_create_reference_table(dadb_interface, annotation_metadata): voxel_resolution_z=vz, table_metadata=table_metadata, flat_segmentation_source=None, - with_crud_columns=False, + with_crud_columns=True, ) assert table_name == table @@ -83,6 +82,35 @@ def test_create_reference_table(dadb_interface, annotation_metadata): assert table_info["reference_table"] == "anno_test" +def test_create_nested_reference_table(dadb_interface, annotation_metadata): + table_name = "reference_tag" + schema_type = "reference_tag" + vx = annotation_metadata["voxel_resolution_x"] + vy = annotation_metadata["voxel_resolution_y"] + vz = annotation_metadata["voxel_resolution_z"] + + table_metadata = { + "reference_table": "presynaptic_bouton_types", + "track_target_id_updates": True, + } + table = dadb_interface.annotation.create_table( + table_name, + schema_type, + description="tags on 'presynaptic_bouton_types' table", + user_id="foo@bar.com", + voxel_resolution_x=vx, + voxel_resolution_y=vy, + voxel_resolution_z=vz, + table_metadata=table_metadata, + flat_segmentation_source=None, + with_crud_columns=True, + ) + assert table_name == table + + table_info = dadb_interface.database.get_table_metadata(table) + assert table_info["reference_table"] == "presynaptic_bouton_types" + + def test_bad_schema_reference_table(dadb_interface, annotation_metadata): table_name = "bad_reference_table" schema_type = "synapse" @@ -138,6 +166,20 @@ def test_insert_reference_annotation(dadb_interface, annotation_metadata): assert inserted_id == [1] +def test_insert_nested_reference_tag_annotation(dadb_interface, annotation_metadata): + table_name = "reference_tag" + + test_data = [ + { + "tag": "here is a tag", + "target_id": 1, + } + ] + inserted_id = dadb_interface.annotation.insert_annotations(table_name, test_data) + + assert inserted_id == [1] + + def test_insert_another_annotation(dadb_interface, annotation_metadata): table_name = annotation_metadata["table_name"] @@ -154,12 +196,22 @@ def test_insert_another_annotation(dadb_interface, annotation_metadata): assert inserted_id == [2] -def test_get_annotation(dadb_interface, annotation_metadata): +def test_get_valid_annotation(dadb_interface, annotation_metadata): table_name = annotation_metadata["table_name"] test_data = dadb_interface.annotation.get_annotations(table_name, [1]) logging.info(test_data) assert test_data[0]["id"] == 1 + assert test_data[0]["valid"] is True + + +def test_get_reference_annotation(dadb_interface, annotation_metadata): + table_name = "presynaptic_bouton_types" + test_data = dadb_interface.annotation.get_annotations(table_name, [1]) + logging.info(test_data) + + assert test_data[0]["id"] == 1 + assert test_data[0]["target_id"] == 1 def test_update_annotation(dadb_interface, annotation_metadata): @@ -180,13 +232,22 @@ def test_update_annotation(dadb_interface, annotation_metadata): assert test_data[0]["superceded_id"] == 3 -def test_get_reference_annotation(dadb_interface, annotation_metadata): +def test_get_not_valid_annotation(dadb_interface, annotation_metadata): + table_name = annotation_metadata["table_name"] + test_data = dadb_interface.annotation.get_annotations(table_name, [1]) + logging.info(test_data) + + assert test_data[0]["id"] == 1 + assert test_data[0]["valid"] is False + + +def test_get_reference_annotation_again(dadb_interface, annotation_metadata): table_name = "presynaptic_bouton_types" test_data = dadb_interface.annotation.get_annotations(table_name, [1]) logging.info(test_data) assert test_data[0]["id"] == 1 - assert test_data[0]["target_id"] == 3 + assert test_data[0]["target_id"] == 1 def test_update_reference_annotation(dadb_interface, annotation_metadata): @@ -195,20 +256,36 @@ def test_update_reference_annotation(dadb_interface, annotation_metadata): test_data = { "id": 1, "bouton_type": "basmati", - "target_id": 3, } update_map = dadb_interface.annotation.update_annotation(table_name, test_data) - assert update_map == {1: 1} - test_data = dadb_interface.annotation.get_annotations(table_name, [1]) + assert update_map == {1: 2} + # return values from newly updated row + test_data = dadb_interface.annotation.get_annotations(table_name, [2]) assert test_data[0]["bouton_type"] == "basmati" +def test_nested_update_reference_annotation(dadb_interface, annotation_metadata): + table_name = "reference_tag" + + test_data = { + "tag": "here is a updated tag", + "id": 1, + } + + update_map = dadb_interface.annotation.update_annotation(table_name, test_data) + + assert update_map == {1: 2} + # return values from newly updated row + test_data = dadb_interface.annotation.get_annotations(table_name, [2]) + assert test_data[0]["tag"] == "here is a updated tag" + + def test_delete_reference_annotation(dadb_interface, annotation_metadata): table_name = "presynaptic_bouton_types" - ids_to_delete = [1] + ids_to_delete = [2] is_deleted = dadb_interface.annotation.delete_annotation(table_name, ids_to_delete) assert is_deleted == ids_to_delete @@ -217,7 +294,7 @@ def test_delete_reference_annotation(dadb_interface, annotation_metadata): def test_delete_annotation(dadb_interface, annotation_metadata): table_name = annotation_metadata["table_name"] - ids_to_delete = [1] + ids_to_delete = [3] is_deleted = dadb_interface.annotation.delete_annotation(table_name, ids_to_delete) assert is_deleted == ids_to_delete diff --git a/tests/test_database.py b/tests/test_database.py index 12d5027..f405a51 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -79,7 +79,7 @@ def test_get_table_valid_row_count(dadb_interface, annotation_metadata): result = dadb_interface.database.get_table_row_count(table_name, filter_valid=True) logging.info(f"{table_name} valid row count: {result}") - assert result == 2 + assert result == 1 def test_get_table_valid_timestamp_row_count(dadb_interface, annotation_metadata):