From 2ab1de82ca7eb43ed2dca2539af9c83749d6c257 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli <26835404+GianlucaFicarelli@users.noreply.github.com> Date: Wed, 14 May 2025 11:28:53 +0200 Subject: [PATCH 1/9] Import and expose derivation between ElectricalCellRecording and EModel --- ..._653df0eee898_default_migration_message.py | 45 +++++ app/cli/import_data.py | 78 +++++++- app/cli/utils.py | 15 +- app/db/model.py | 8 + app/filters/electrical_cell_recording.py | 16 +- app/queries/factory.py | 4 + app/routers/emodel.py | 30 +++ app/service/electrical_cell_recording.py | 12 +- tests/conftest.py | 24 +++ tests/test_electrical_cell_recording.py | 172 ++++++++---------- tests/test_emodel.py | 27 ++- tests/utils.py | 60 +++++- 12 files changed, 373 insertions(+), 118 deletions(-) create mode 100644 alembic/versions/20250526_142504_653df0eee898_default_migration_message.py diff --git a/alembic/versions/20250526_142504_653df0eee898_default_migration_message.py b/alembic/versions/20250526_142504_653df0eee898_default_migration_message.py new file mode 100644 index 00000000..c8b13f2c --- /dev/null +++ b/alembic/versions/20250526_142504_653df0eee898_default_migration_message.py @@ -0,0 +1,45 @@ +"""Default migration message + +Revision ID: 653df0eee898 +Revises: 698536102bc8 +Create Date: 2025-05-26 14:25:04.120757 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +from sqlalchemy import Text +import app.db.types + +# revision identifiers, used by Alembic. +revision: str = "653df0eee898" +down_revision: Union[str, None] = "698536102bc8" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + "derivation", + sa.Column("used_id", sa.Uuid(), nullable=False), + sa.Column("generated_id", sa.Uuid(), nullable=False), + sa.ForeignKeyConstraint( + ["generated_id"], ["entity.id"], name=op.f("fk_derivation_generated_id_entity") + ), + sa.ForeignKeyConstraint( + ["used_id"], ["entity.id"], name=op.f("fk_derivation_used_id_entity") + ), + sa.PrimaryKeyConstraint("used_id", "generated_id", name=op.f("pk_derivation")), + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("derivation") + # ### end Alembic commands ### diff --git a/app/cli/import_data.py b/app/cli/import_data.py index 1b8e49ce..37ace65a 100644 --- a/app/cli/import_data.py +++ b/app/cli/import_data.py @@ -7,7 +7,6 @@ from abc import ABC, abstractmethod from collections import Counter, defaultdict from contextlib import closing -from operator import attrgetter from pathlib import Path from typing import Any @@ -18,7 +17,7 @@ from app.cli import curate, utils from app.cli.brain_region_data import BRAIN_ATLAS_REGION_VOLUMES -from app.cli.curation import electrical_cell_recording +from app.cli.curation import cell_composition, electrical_cell_recording from app.cli.types import ContentType from app.cli.utils import ( AUTHORIZED_PUBLIC, @@ -37,6 +36,7 @@ BrainRegionHierarchy, CellComposition, DataMaturityAnnotationBody, + Derivation, ElectricalCellRecording, EModel, Entity, @@ -70,10 +70,6 @@ from app.logger import L from app.schemas.base import ProjectContext -from app.cli.curation import electrical_cell_recording, cell_composition -from app.cli.types import ContentType - - BRAIN_ATLAS_NAME = "BlueBrain Atlas" REQUIRED_PATH = click.Path(exists=True, readable=True, dir_okay=False, resolve_path=True) @@ -560,6 +556,75 @@ def ingest( db.commit() +class ImportEModelDerivations(Import): + name = "EModelWorkflow" + + @staticmethod + def is_correct_type(data): + types = ensurelist(data["@type"]) + return "EModelWorkflow" in types + + @staticmethod + def ingest( + db, + project_context, + data_list: list[dict], + all_data_by_id: dict[str, dict], + hierarchy_name: str, + ): + """Import emodel derivations from EModelWorkflow.""" + legacy_emodel_ids = set() + derivations = {} + for data in tqdm(data_list, desc="EModelWorkflow"): + legacy_emodel_id = utils.find_id_in_entity(data, "EModel", "generates") + legacy_etc_id = utils.find_id_in_entity( + data, "ExtractionTargetsConfiguration", "hasPart" + ) + if not legacy_emodel_id: + L.warning("Not found EModel id in EModelWorkflow: {}", data["@id"]) + continue + if not legacy_etc_id: + L.warning( + "Not found ExtractionTargetsConfiguration id in EModelWorkflow: {}", data["@id"] + ) + continue + if not (etc := all_data_by_id.get(legacy_etc_id)): + L.warning("Not found ExtractionTargetsConfiguration with id {}", legacy_etc_id) + continue + if not (legacy_trace_ids := list(utils.find_ids_in_entity(etc, "Trace", "uses"))): + L.warning( + "Not found traces in ExtractionTargetsConfiguration with id {}", legacy_etc_id + ) + continue + if legacy_emodel_id in legacy_emodel_ids: + L.warning("Duplicated and ignored traces for EModel id {}", legacy_emodel_id) + continue + legacy_emodel_ids.add(legacy_emodel_id) + if not (emodel := utils._find_by_legacy_id(legacy_emodel_id, EModel, db)): + L.warning("Not found EModel with legacy id {}", legacy_emodel_id) + continue + if emodel.id in derivations: + L.warning("Duplicated and ignored traces for EModel uuid {}", emodel.id) + derivations[emodel.id] = [ + utils._find_by_legacy_id(legacy_trace_id, ElectricalCellRecording, db).id + for legacy_trace_id in legacy_trace_ids + ] + + rows = [ + Derivation(used_id=trace_id, generated_id=emodel_id) + for emodel_id, trace_ids in derivations.items() + for trace_id in trace_ids + ] + L.info( + "Imported derivations for {} EModels from {} records", len(derivations), len(data_list) + ) + # delete everything from derivation table before adding the records + query = sa.delete(Derivation) + db.execute(query) + db.add_all(rows) + db.commit() + + class ImportBrainAtlas(Import): name = "BrainAtlas" @@ -1359,6 +1424,7 @@ def _do_import(db, input_dir, project_context, hierarchy_name): ImportBrainAtlas, ImportDistribution, ImportNeuronMorphologyFeatureAnnotation, + ImportEModelDerivations, ] for importer in importers: diff --git a/app/cli/utils.py b/app/cli/utils.py index 4e2b3f8f..2a668559 100644 --- a/app/cli/utils.py +++ b/app/cli/utils.py @@ -363,13 +363,14 @@ def get_or_create_distribution( def find_id_in_entity(entity: dict | None, type_: str, entity_list_key: str): if not entity: return None - return next( - ( - part.get("@id") - for part in ensurelist(entity.get(entity_list_key, [])) - if is_type(part, type_) - ), - None, + return next(find_ids_in_entity(entity, type_, entity_list_key), None) + + +def find_ids_in_entity(entity: dict, type_: str, entity_list_key: str): + return ( + part.get("@id") + for part in ensurelist(entity.get(entity_list_key, [])) + if is_type(part, type_) ) diff --git a/app/db/model.py b/app/db/model.py index 105cdfad..91cfa98f 100644 --- a/app/db/model.py +++ b/app/db/model.py @@ -896,3 +896,11 @@ class CellComposition(NameDescriptionVectorMixin, LocationMixin, SpeciesMixin, E __tablename__ = EntityType.cell_composition id: Mapped[uuid.UUID] = mapped_column(ForeignKey("entity.id"), primary_key=True) __mapper_args__ = {"polymorphic_identity": __tablename__} # noqa: RUF012 + + +class Derivation(Base): + __tablename__ = "derivation" + used_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("entity.id"), primary_key=True) + generated_id: Mapped[uuid.UUID] = mapped_column(ForeignKey("entity.id"), primary_key=True) + used: Mapped["Entity"] = relationship(foreign_keys=[used_id]) + generated: Mapped["Entity"] = relationship(foreign_keys=[generated_id]) diff --git a/app/filters/electrical_cell_recording.py b/app/filters/electrical_cell_recording.py index bf780317..d33d6e6e 100644 --- a/app/filters/electrical_cell_recording.py +++ b/app/filters/electrical_cell_recording.py @@ -1,20 +1,32 @@ from typing import Annotated -from fastapi_filter import FilterDepends +from fastapi_filter import FilterDepends, with_prefix -from app.db.model import ElectricalCellRecording +from app.db.model import ElectricalCellRecording, EModel from app.filters.base import CustomFilter from app.filters.common import ( BrainRegionFilterMixin, EntityFilterMixin, + IdFilterMixin, NameFilterMixin, SubjectFilterMixin, ) +class _NestedEModelFilter(IdFilterMixin, CustomFilter): + """Simplified EModel filter allowing to filter only by id and id__in.""" + + class Constants(CustomFilter.Constants): + model = EModel + + class ElectricalCellRecordingFilter( CustomFilter, BrainRegionFilterMixin, SubjectFilterMixin, EntityFilterMixin, NameFilterMixin ): + generated_emodel: Annotated[ + _NestedEModelFilter | None, + FilterDepends(with_prefix("generated_emodel", _NestedEModelFilter)), + ] = None order_by: list[str] = ["-creation_date"] # noqa: RUF012 class Constants(CustomFilter.Constants): diff --git a/app/queries/factory.py b/app/queries/factory.py index 8ac4bbd8..0a45b946 100644 --- a/app/queries/factory.py +++ b/app/queries/factory.py @@ -4,6 +4,7 @@ Agent, BrainRegion, Contribution, + Derivation, EModel, ETypeClass, ETypeClassification, @@ -106,6 +107,9 @@ def _get_alias[T: type[Identifiable]](db_cls: T, name: str | None = None) -> T: morphology_alias, db_model_class.exemplar_morphology_id == morphology_alias.id ), "emodel": lambda q: q.join(emodel_alias, db_model_class.emodel_id == emodel_alias.id), + "generated_emodel": lambda q: q.join( + Derivation, Derivation.used_id == db_model_class.id + ).join(emodel_alias, Derivation.generated_id == emodel_alias.id), "me_model": lambda q: q.join( me_model_alias, db_model_class.me_model_id == me_model_alias.id ), diff --git a/app/routers/emodel.py b/app/routers/emodel.py index 49b5fed0..512ddbd4 100644 --- a/app/routers/emodel.py +++ b/app/routers/emodel.py @@ -1,6 +1,15 @@ +import uuid + from fastapi import APIRouter +import app.service.electrical_cell_recording import app.service.emodel +from app.dependencies.auth import UserContextDep +from app.dependencies.common import PaginationQuery +from app.dependencies.db import SessionDep +from app.filters.electrical_cell_recording import ElectricalCellRecordingFilter +from app.schemas.electrical_cell_recording import ElectricalCellRecordingRead +from app.schemas.types import ListResponse router = APIRouter( prefix="/emodel", @@ -10,3 +19,24 @@ read_many = router.get("")(app.service.emodel.read_many) read_one = router.get("/{id_}")(app.service.emodel.read_one) create_one = router.post("")(app.service.emodel.create_one) + + +@router.get("/{id_}/electrical-cell-recording") +def get_electrical_cell_recording( + *, + user_context: UserContextDep, + db: SessionDep, + id_: uuid.UUID, + pagination_request: PaginationQuery, +) -> ListResponse[ElectricalCellRecordingRead]: + """Return the list of ElectricalCellRecording used to generate the specified EModel.""" + filter_model = ElectricalCellRecordingFilter.model_validate({"generated_emodel": {"id": id_}}) + return app.service.electrical_cell_recording.read_many( + user_context=user_context, + db=db, + pagination_request=pagination_request, + filter_model=filter_model, + with_search=None, + facets=None, + in_brain_region=None, + ) diff --git a/app/service/electrical_cell_recording.py b/app/service/electrical_cell_recording.py index dd9d5056..ad7d492f 100644 --- a/app/service/electrical_cell_recording.py +++ b/app/service/electrical_cell_recording.py @@ -7,6 +7,7 @@ from app.db.model import ( Agent, ElectricalCellRecording, + EModel, Subject, ) from app.dependencies.auth import UserContextDep, UserContextWithProjectIdDep @@ -85,19 +86,28 @@ def read_many( agent_alias = aliased(Agent, flat=True) created_by_alias = aliased(Agent, flat=True) updated_by_alias = aliased(Agent, flat=True) + emodel_alias = aliased(EModel, flat=True) aliases: Aliases = { Agent: { "contribution": agent_alias, "created_by": created_by_alias, "updated_by": updated_by_alias, }, + EModel: emodel_alias, } - facet_keys = filter_keys = [ + facet_keys = [ "brain_region", "created_by", "updated_by", "contribution", ] + filter_keys = [ + "brain_region", + "created_by", + "updated_by", + "contribution", + "generated_emodel", + ] name_to_facet_query_params, filter_joins = query_params_factory( db_model_class=ElectricalCellRecording, facet_keys=facet_keys, diff --git a/tests/conftest.py b/tests/conftest.py index 38173532..6107212d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -55,6 +55,7 @@ ClientProxy, add_db, assert_request, + create_electrical_cell_recording_id_with_assets, ) @@ -671,3 +672,26 @@ def faceted_memodels(db: Session, client: TestClient, agents: tuple[Agent, Agent brain_region_ids=brain_region_ids, agent_ids=agent_ids, ) + + +@pytest.fixture +def electrical_cell_recording_json_data(brain_region_id, subject_id, license_id): + return { + "name": "my-name", + "description": "my-description", + "subject_id": subject_id, + "brain_region_id": str(brain_region_id), + "license_id": str(license_id), + "recording_location": ["soma[0]_0.5"], + "recording_type": "intracellular", + "recording_origin": "in_vivo", + "ljp": 11.5, + "authorized_public": False, + } + + +@pytest.fixture +def trace_id_with_assets(db, client, tmp_path, electrical_cell_recording_json_data): + return create_electrical_cell_recording_id_with_assets( + db, client, tmp_path, electrical_cell_recording_json_data + ) diff --git a/tests/test_electrical_cell_recording.py b/tests/test_electrical_cell_recording.py index 25f46dc9..952a2509 100644 --- a/tests/test_electrical_cell_recording.py +++ b/tests/test_electrical_cell_recording.py @@ -2,113 +2,32 @@ import pytest -from app.db.model import ElectricalCellRecording, ElectricalRecordingStimulus +from app.db.model import Derivation from app.db.types import EntityType from .utils import ( PROJECT_ID, - add_db, + add_all_db, assert_request, + assert_response, check_authorization, check_brain_region_filter, check_missing, - create_asset_file, create_brain_region, + create_electrical_cell_recording_db, + create_electrical_cell_recording_id, ) -MODEL = ElectricalCellRecording ROUTE = "electrical-cell-recording" -@pytest.fixture -def json_data(brain_region_id, subject_id, license_id): - return { - "name": "my-name", - "description": "my-description", - "subject_id": subject_id, - "brain_region_id": str(brain_region_id), - "license_id": str(license_id), - "recording_location": ["soma[0]_0.5"], - "recording_type": "intracellular", - "recording_origin": "in_vivo", - "ljp": 11.5, - "authorized_public": False, - } - - -@pytest.fixture -def create(client, json_data): - def _create(**kwargs): - return assert_request(client.post, url=ROUTE, json=json_data | kwargs).json() - - return _create - - -@pytest.fixture -def create_id(create): - def _create_id(**kwargs): - return create(**kwargs)["id"] - - return _create_id - - -@pytest.fixture -def create_db(db, create_id): - def _create_db(**kwargs): - return db.get(MODEL, create_id(**kwargs)) - - return _create_db - - -def _create_electrical_recording_id( - db, - recording_id, +def test_create_one( + client, subject_id, license_id, brain_region_id, electrical_cell_recording_json_data ): - return add_db( - db, - ElectricalRecordingStimulus( - name="protocol", - description="protocol-description", - dt=0.1, - injection_type="current_clamp", - shape="sinusoidal", - start_time=0.0, - end_time=1.0, - recording_id=recording_id, - authorized_public=False, - authorized_project_id=PROJECT_ID, - ), - ).id - - -@pytest.fixture -def trace_id(tmp_path, client, db, create_id): - trace_id = create_id() - - # add two protocols that refer to it - _create_electrical_recording_id(db, trace_id) - _create_electrical_recording_id(db, trace_id) - - filepath = tmp_path / "trace.nwb" - filepath.write_bytes(b"trace") - - # add an asset too - create_asset_file( - client=client, - entity_type="electrical_cell_recording", - entity_id=trace_id, - file_name="my-trace.nwb", - file_obj=filepath.read_bytes(), - ) - - return trace_id - - -def test_create_one(client, subject_id, license_id, brain_region_id, json_data): data = assert_request( client.post, url=ROUTE, - json=json_data, + json=electrical_cell_recording_json_data, ).json() assert data["name"] == "my-name" @@ -121,10 +40,10 @@ def test_create_one(client, subject_id, license_id, brain_region_id, json_data): assert data["created_by"]["id"] == data["updated_by"]["id"] -def test_read_one(client, subject_id, license_id, brain_region_id, trace_id): +def test_read_one(client, subject_id, license_id, brain_region_id, trace_id_with_assets): data = assert_request( client.get, - url=f"{ROUTE}/{trace_id}", + url=f"{ROUTE}/{trace_id_with_assets}", ).json() assert data["name"] == "my-name" @@ -144,12 +63,21 @@ def test_missing(client): check_missing(ROUTE, client) -def test_authorization(client_user_1, client_user_2, client_no_project, json_data): - check_authorization(ROUTE, client_user_1, client_user_2, client_no_project, json_data) +def test_authorization( + client_user_1, client_user_2, client_no_project, electrical_cell_recording_json_data +): + check_authorization( + ROUTE, client_user_1, client_user_2, client_no_project, electrical_cell_recording_json_data + ) -def test_pagination(client, create_id): - _ = [create_id(name=f"entity-{i}") for i in range(2)] +def test_pagination(client, electrical_cell_recording_json_data): + _ = [ + create_electrical_cell_recording_id( + client, json_data=electrical_cell_recording_json_data | {"name": f"entity-{i}"} + ) + for i in range(2) + ] response = assert_request( client.get, url=ROUTE, @@ -163,7 +91,7 @@ def test_pagination(client, create_id): @pytest.fixture -def faceted_ids(db, brain_region_hierarchy_id, create_id): +def faceted_ids(db, client, brain_region_hierarchy_id, electrical_cell_recording_json_data): brain_region_ids = [ create_brain_region( db, brain_region_hierarchy_id, annotation_value=i, name=f"region-{i}" @@ -172,8 +100,14 @@ def faceted_ids(db, brain_region_hierarchy_id, create_id): ] trace_ids = [ - create_id( - name=f"trace-{i}", description=f"brain-region-{i}", brain_region_id=str(region_id) + create_electrical_cell_recording_id( + client, + json_data=electrical_cell_recording_json_data + | { + "name": f"trace-{i}", + "description": f"brain-region-{i}", + "brain_region_id": str(region_id), + }, ) for i, region_id in enumerate(brain_region_ids) ] @@ -225,8 +159,46 @@ def test_facets(client, faceted_ids): ] -def test_brain_region_filter(db, client, brain_region_hierarchy_id, create_db): +def test_brain_region_filter( + db, client, brain_region_hierarchy_id, electrical_cell_recording_json_data +): def create_model_function(_db, name, brain_region_id): - return create_db(name=name, brain_region_id=str(brain_region_id)) + return create_electrical_cell_recording_db( + db, + client, + json_data=electrical_cell_recording_json_data + | {"name": name, "brain_region_id": str(brain_region_id)}, + ) check_brain_region_filter(ROUTE, client, db, brain_region_hierarchy_id, create_model_function) + + +def test_get_traces_for_generated_emodel( + db, client, create_emodel_ids, electrical_cell_recording_json_data +): + # create two emodels, one with derivations and one without + generated_emodel_id, other_emodel_id = create_emodel_ids(2) + trace_ids = [ + create_electrical_cell_recording_id( + client, json_data=electrical_cell_recording_json_data | {"name": f"name-{i}"} + ) + for i in range(2) + ] + derivations = [ + Derivation(used_id=ecr_id, generated_id=generated_emodel_id) for ecr_id in trace_ids + ] + add_all_db(db, derivations) + + response = client.get(url=ROUTE, params={"generated_emodel__id": generated_emodel_id}) + + assert_response(response, 200) + data = response.json()["data"] + assert len(data) == 2 + assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} + assert all(d["type"] == "electrical_cell_recording" for d in data) + + response = client.get(url=ROUTE, params={"generated_emodel__id": other_emodel_id}) + + assert_response(response, 200) + data = response.json()["data"] + assert len(data) == 0 diff --git a/tests/test_emodel.py b/tests/test_emodel.py index 75de7c9f..e28d345d 100644 --- a/tests/test_emodel.py +++ b/tests/test_emodel.py @@ -3,10 +3,16 @@ from fastapi.testclient import TestClient +from app.db.model import Derivation from app.db.types import EntityType from .conftest import CreateIds, EModelIds -from .utils import create_reconstruction_morphology_id +from .utils import ( + add_all_db, + assert_response, + create_electrical_cell_recording_id, + create_reconstruction_morphology_id, +) from tests.routers.test_asset import _upload_entity_asset ROUTE = "/emodel" @@ -365,3 +371,22 @@ def test_pagination(client, create_emodel_ids): assert len(items) == total_items data_ids = [int(i["name"]) for i in items] assert list(reversed(data_ids)) == list(range(total_items)) + + +def test_get_electrical_cell_recording(client, db, emodel_id, electrical_cell_recording_json_data): + trace_ids = [ + create_electrical_cell_recording_id( + client, json_data=electrical_cell_recording_json_data | {"name": f"name-{i}"} + ) + for i in range(2) + ] + derivations = [Derivation(used_id=ecr_id, generated_id=emodel_id) for ecr_id in trace_ids] + add_all_db(db, derivations) + + response = client.get(f"{ROUTE}/{emodel_id}/electrical-cell-recording") + + assert_response(response, 200) + data = response.json()["data"] + assert len(data) == 2 + assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} + assert all(d["type"] == "electrical_cell_recording" for d in data) diff --git a/tests/utils.py b/tests/utils.py index f3dfec50..aadf8d22 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,8 +10,11 @@ from app.db.model import ( BrainRegion, BrainRegionHierarchy, + ElectricalCellRecording, + ElectricalRecordingStimulus, MTypeClass, MTypeClassification, + ReconstructionMorphology, ) from app.db.types import EntityType @@ -42,6 +45,11 @@ "project-id": UNRELATED_PROJECT_ID, } +ROUTES = { + ReconstructionMorphology: "/reconstruction-morphology", + ElectricalCellRecording: "/electrical-cell-recording", +} + class ClientProxy: """Proxy TestClient to pass default headers without creating a new instance. @@ -78,7 +86,7 @@ def create_reconstruction_morphology_id( description="Test Morphology Description", ): response = client.post( - "/reconstruction-morphology", + ROUTES[ReconstructionMorphology], json={ "name": name, "description": description, @@ -162,6 +170,56 @@ def attach_mtype(db, entity_id, mtype_id): return add_db(db, MTypeClassification(entity_id=str(entity_id), mtype_class_id=str(mtype_id))) +def create_electrical_recording_stimulus_id(db, recording_id): + return add_db( + db, + ElectricalRecordingStimulus( + name="protocol", + description="protocol-description", + dt=0.1, + injection_type="current_clamp", + shape="sinusoidal", + start_time=0.0, + end_time=1.0, + recording_id=recording_id, + authorized_public=False, + authorized_project_id=PROJECT_ID, + ), + ).id + + +def create_electrical_cell_recording_id(client, json_data): + result = assert_request(client.post, url=ROUTES[ElectricalCellRecording], json=json_data).json() + return uuid.UUID(result["id"]) + + +def create_electrical_cell_recording_db(db, client, json_data): + trace_id = create_electrical_cell_recording_id(client, json_data) + return db.get(ElectricalCellRecording, trace_id) + + +def create_electrical_cell_recording_id_with_assets(db, client, tmp_path, json_data): + trace_id = create_electrical_cell_recording_id(client, json_data) + + # add two protocols that refer to it + create_electrical_recording_stimulus_id(db, trace_id) + create_electrical_recording_stimulus_id(db, trace_id) + + filepath = tmp_path / "trace.nwb" + filepath.write_bytes(b"trace") + + # add an asset too + create_asset_file( + client=client, + entity_type="electrical_cell_recording", + entity_id=trace_id, + file_name="my-trace.nwb", + file_obj=filepath.read_bytes(), + ) + + return trace_id + + def check_missing(route, client): assert_request( client.get, From 2cd4ae8658bc8d23306e73a8f681278f7864a23f Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Tue, 27 May 2025 10:47:13 +0200 Subject: [PATCH 2/9] Add EModel.electrical_cell_recordings --- app/db/model.py | 6 ++++++ app/schemas/base.py | 4 ++++ app/schemas/emodel.py | 2 ++ app/service/emodel.py | 1 + tests/test_emodel.py | 1 + 5 files changed, 14 insertions(+) diff --git a/app/db/model.py b/app/db/model.py index 91cfa98f..baf1098f 100644 --- a/app/db/model.py +++ b/app/db/model.py @@ -459,6 +459,12 @@ class EModel( viewonly=True, order_by="IonChannelModel.creation_date", ) + electrical_cell_recordings: Mapped[list["ElectricalCellRecording"]] = relationship( + primaryjoin="EModel.id == Derivation.generated_id", + secondaryjoin="Entity.id == Derivation.used_id", + secondary="derivation", + viewonly=True, + ) __mapper_args__ = {"polymorphic_identity": __tablename__} # noqa: RUF012 diff --git a/app/schemas/base.py b/app/schemas/base.py index d1efb98d..f9fd1b61 100644 --- a/app/schemas/base.py +++ b/app/schemas/base.py @@ -109,3 +109,7 @@ class LicensedCreateMixin(BaseModel): class LicensedReadMixin(BaseModel): model_config = ConfigDict(from_attributes=True) license: LicenseRead | None + + +class BasicEntityRead(IdentifiableMixin, EntityTypeMixin): + pass diff --git a/app/schemas/emodel.py b/app/schemas/emodel.py index 580998b9..ff989c78 100644 --- a/app/schemas/emodel.py +++ b/app/schemas/emodel.py @@ -8,6 +8,7 @@ from app.schemas.base import ( AuthorizationMixin, AuthorizationOptionalPublicMixin, + BasicEntityRead, BrainRegionRead, CreationMixin, EntityTypeMixin, @@ -60,3 +61,4 @@ class EModelRead( class EModelReadExpanded(EModelRead, AssetsMixin): ion_channel_models: list[IonChannelModelWAssets] + electrical_cell_recordings: list[BasicEntityRead] diff --git a/app/service/emodel.py b/app/service/emodel.py index 131071f3..377c3904 100644 --- a/app/service/emodel.py +++ b/app/service/emodel.py @@ -45,6 +45,7 @@ def _load(select: sa.Select[tuple[EModel]]): selectinload(EModel.ion_channel_models).joinedload(IonChannelModel.strain), selectinload(EModel.ion_channel_models).joinedload(IonChannelModel.brain_region), selectinload(EModel.ion_channel_models).selectinload(IonChannelModel.assets), + selectinload(EModel.electrical_cell_recordings), joinedload(EModel.created_by), joinedload(EModel.updated_by), raiseload("*"), diff --git a/tests/test_emodel.py b/tests/test_emodel.py index e28d345d..d286e2f9 100644 --- a/tests/test_emodel.py +++ b/tests/test_emodel.py @@ -59,6 +59,7 @@ def test_get_emodel(client: TestClient, emodel_id: str): assert "assets" in data assert len(data["assets"]) == 1 assert "ion_channel_models" in data + assert "electrical_cell_recordings" in data assert data["created_by"]["id"] == data["updated_by"]["id"] From 792fe6b252e00fa435a032d1bb7789f663a12732 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Tue, 27 May 2025 11:27:29 +0200 Subject: [PATCH 3/9] Add EModel.electrical_cell_recordings --- app/routers/emodel.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/routers/emodel.py b/app/routers/emodel.py index 512ddbd4..d4b764d2 100644 --- a/app/routers/emodel.py +++ b/app/routers/emodel.py @@ -5,7 +5,7 @@ import app.service.electrical_cell_recording import app.service.emodel from app.dependencies.auth import UserContextDep -from app.dependencies.common import PaginationQuery +from app.dependencies.common import InBrainRegionQuery, PaginationQuery, Search, WithFacets from app.dependencies.db import SessionDep from app.filters.electrical_cell_recording import ElectricalCellRecordingFilter from app.schemas.electrical_cell_recording import ElectricalCellRecordingRead @@ -36,7 +36,7 @@ def get_electrical_cell_recording( db=db, pagination_request=pagination_request, filter_model=filter_model, - with_search=None, - facets=None, - in_brain_region=None, + with_search=Search(), + facets=WithFacets(), + in_brain_region=InBrainRegionQuery(), ) From c4c8519b3c63b5a65ca0d307d3e04d0ffbcd1779 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Tue, 27 May 2025 11:46:29 +0200 Subject: [PATCH 4/9] Rebase and update migration --- ...7_114536_d20b8917d3ab_default_migration_message.py} | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) rename alembic/versions/{20250526_142504_653df0eee898_default_migration_message.py => 20250527_114536_d20b8917d3ab_default_migration_message.py} (86%) diff --git a/alembic/versions/20250526_142504_653df0eee898_default_migration_message.py b/alembic/versions/20250527_114536_d20b8917d3ab_default_migration_message.py similarity index 86% rename from alembic/versions/20250526_142504_653df0eee898_default_migration_message.py rename to alembic/versions/20250527_114536_d20b8917d3ab_default_migration_message.py index c8b13f2c..2404815d 100644 --- a/alembic/versions/20250526_142504_653df0eee898_default_migration_message.py +++ b/alembic/versions/20250527_114536_d20b8917d3ab_default_migration_message.py @@ -1,8 +1,8 @@ """Default migration message -Revision ID: 653df0eee898 -Revises: 698536102bc8 -Create Date: 2025-05-26 14:25:04.120757 +Revision ID: d20b8917d3ab +Revises: 8d1610d7c882 +Create Date: 2025-05-27 11:45:36.409166 """ @@ -16,8 +16,8 @@ import app.db.types # revision identifiers, used by Alembic. -revision: str = "653df0eee898" -down_revision: Union[str, None] = "698536102bc8" +revision: str = "d20b8917d3ab" +down_revision: Union[str, None] = "8d1610d7c882" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None From 6604e0dada2594dbbc4971399bb2023c5ec13b68 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Tue, 27 May 2025 16:29:09 +0200 Subject: [PATCH 5/9] Add endpoint /{entity_route}/{entity_id}/generated-by --- app/filters/entity.py | 20 +++++++++++++++ app/routers/__init__.py | 2 ++ app/routers/asset.py | 28 ++++++-------------- app/routers/derivation.py | 12 +++++++++ app/service/derivation.py | 54 +++++++++++++++++++++++++++++++++++++++ app/utils/routers.py | 16 ++++++++++++ tests/test_derivation.py | 34 ++++++++++++++++++++++++ 7 files changed, 146 insertions(+), 20 deletions(-) create mode 100644 app/filters/entity.py create mode 100644 app/routers/derivation.py create mode 100644 app/service/derivation.py create mode 100644 app/utils/routers.py create mode 100644 tests/test_derivation.py diff --git a/app/filters/entity.py b/app/filters/entity.py new file mode 100644 index 00000000..066ac67a --- /dev/null +++ b/app/filters/entity.py @@ -0,0 +1,20 @@ +from typing import Annotated + +from fastapi_filter import FilterDepends + +from app.db.model import Entity +from app.db.types import EntityType +from app.filters.base import CustomFilter + + +class BasicEntityFilter(CustomFilter): + type: EntityType | None = None + + order_by: list[str] = ["-creation_date"] # noqa: RUF012 + + class Constants(CustomFilter.Constants): + model = Entity + ordering_model_fields = ["creation_date", "update_date", "name"] # noqa: RUF012 + + +BasicEntityFilterDep = Annotated[BasicEntityFilter, FilterDepends(BasicEntityFilter)] diff --git a/app/routers/__init__.py b/app/routers/__init__.py index 255dd5b0..f2081251 100644 --- a/app/routers/__init__.py +++ b/app/routers/__init__.py @@ -10,6 +10,7 @@ brain_region_hierarchy, cell_composition, contribution, + derivation, electrical_cell_recording, emodel, etype, @@ -44,6 +45,7 @@ brain_region_hierarchy.router, cell_composition.router, contribution.router, + derivation.router, electrical_cell_recording.router, emodel.router, etype.router, diff --git a/app/routers/asset.py b/app/routers/asset.py index e8d84c00..d00016a0 100644 --- a/app/routers/asset.py +++ b/app/routers/asset.py @@ -1,16 +1,15 @@ """Generic asset routes.""" import uuid -from enum import StrEnum from http import HTTPStatus from pathlib import Path -from typing import TYPE_CHECKING, Annotated +from typing import Annotated from fastapi import APIRouter, Form, HTTPException, UploadFile, status from starlette.responses import RedirectResponse from app.config import settings -from app.db.types import AssetLabel, EntityType +from app.db.types import AssetLabel from app.dependencies.auth import UserContextDep, UserContextWithProjectIdDep from app.dependencies.db import RepoGroupDep from app.dependencies.s3 import S3ClientDep @@ -19,6 +18,7 @@ from app.schemas.types import ListResponse, PaginationResponse from app.service import asset as asset_service from app.utils.files import calculate_sha256_digest, get_content_type +from app.utils.routers import EntityRoute, entity_route_to_type from app.utils.s3 import ( delete_from_s3, generate_presigned_url, @@ -32,18 +32,6 @@ tags=["assets"], ) -if not TYPE_CHECKING: - # EntityRoute (hyphen-separated) <-> EntityType (underscore_separated) - EntityRoute = StrEnum( - "EntityRoute", {item.name: item.name.replace("_", "-") for item in EntityType} - ) -else: - EntityRoute = StrEnum - - -def _entity_route_to_type(entity_route: EntityRoute) -> EntityType: - return EntityType[entity_route.name] - @router.get("/{entity_route}/{entity_id}/assets") def get_entity_assets( @@ -56,7 +44,7 @@ def get_entity_assets( assets = asset_service.get_entity_assets( repos, user_context=user_context, - entity_type=_entity_route_to_type(entity_route), + entity_type=entity_route_to_type(entity_route), entity_id=entity_id, ) # TODO: proper pagination @@ -76,7 +64,7 @@ def get_entity_asset( return asset_service.get_entity_asset( repos, user_context=user_context, - entity_type=_entity_route_to_type(entity_route), + entity_type=entity_route_to_type(entity_route), entity_id=entity_id, asset_id=asset_id, ) @@ -117,7 +105,7 @@ def upload_entity_asset( asset_read = asset_service.create_entity_asset( repos=repos, user_context=user_context, - entity_type=_entity_route_to_type(entity_route), + entity_type=entity_route_to_type(entity_route), entity_id=entity_id, filename=file.filename, content_type=content_type, @@ -149,7 +137,7 @@ def download_entity_asset( asset = asset_service.get_entity_asset( repos, user_context=user_context, - entity_type=_entity_route_to_type(entity_route), + entity_type=entity_route_to_type(entity_route), entity_id=entity_id, asset_id=asset_id, ) @@ -205,7 +193,7 @@ def delete_entity_asset( asset = asset_service.delete_entity_asset( repos, user_context=user_context, - entity_type=_entity_route_to_type(entity_route), + entity_type=entity_route_to_type(entity_route), entity_id=entity_id, asset_id=asset_id, ) diff --git a/app/routers/derivation.py b/app/routers/derivation.py new file mode 100644 index 00000000..cb5a96c8 --- /dev/null +++ b/app/routers/derivation.py @@ -0,0 +1,12 @@ +"""Generic derivation routes.""" + +from fastapi import APIRouter + +import app.service.derivation + +router = APIRouter( + prefix="", + tags=["derivation"], +) + +router.get("/{entity_route}/{entity_id}/generated-by")(app.service.derivation.read_many) diff --git a/app/service/derivation.py b/app/service/derivation.py new file mode 100644 index 00000000..e7cd3c6a --- /dev/null +++ b/app/service/derivation.py @@ -0,0 +1,54 @@ +"""Generic derivation service.""" + +import uuid + +from sqlalchemy import and_ +from sqlalchemy.orm import aliased + +from app.db.model import Derivation, Entity +from app.dependencies.auth import UserContextDep +from app.dependencies.common import PaginationQuery +from app.dependencies.db import SessionDep +from app.filters.entity import BasicEntityFilterDep +from app.queries.common import router_read_many +from app.schemas.base import BasicEntityRead +from app.schemas.types import ListResponse +from app.utils.routers import EntityRoute, entity_route_to_type + + +def read_many( + *, + user_context: UserContextDep, + db: SessionDep, + entity_route: EntityRoute, + entity_id: uuid.UUID, + pagination_request: PaginationQuery, + entity_filter: BasicEntityFilterDep, +) -> ListResponse[BasicEntityRead]: + """Return a list of basic entities used to generate the specified entity.""" + db_model_class = Entity + generated_alias = aliased(Entity, flat=True, name="generated_alias") + entity_type = entity_route_to_type(entity_route) + # always needed regardless of the filter, so they cannot go to filter_keys + apply_filter_query_operations = ( + lambda q: q.join(Derivation, db_model_class.id == Derivation.used_id) + .join(generated_alias, Derivation.generated_id == generated_alias.id) + .where(and_(generated_alias.id == entity_id, generated_alias.type == entity_type)) + ) + name_to_facet_query_params = filter_joins = None + return router_read_many( + db=db, + db_model_class=db_model_class, + authorized_project_id=user_context.project_id, + with_search=None, + with_in_brain_region=None, + facets=None, + aliases={}, + apply_filter_query_operations=apply_filter_query_operations, + apply_data_query_operations=None, + pagination_request=pagination_request, + response_schema_class=BasicEntityRead, + name_to_facet_query_params=name_to_facet_query_params, + filter_model=entity_filter, + filter_joins=filter_joins, + ) diff --git a/app/utils/routers.py b/app/utils/routers.py new file mode 100644 index 00000000..ffda91b1 --- /dev/null +++ b/app/utils/routers.py @@ -0,0 +1,16 @@ +from enum import StrEnum +from typing import TYPE_CHECKING + +from app.db.types import EntityType + +if not TYPE_CHECKING: + # EntityRoute (hyphen-separated) <-> EntityType (underscore_separated) + EntityRoute = StrEnum( + "EntityRoute", {item.name: item.name.replace("_", "-") for item in EntityType} + ) +else: + EntityRoute = StrEnum + + +def entity_route_to_type(entity_route: EntityRoute) -> EntityType: + return EntityType[entity_route.name] diff --git a/tests/test_derivation.py b/tests/test_derivation.py new file mode 100644 index 00000000..6d4ee5b9 --- /dev/null +++ b/tests/test_derivation.py @@ -0,0 +1,34 @@ +from app.db.model import Derivation + +from tests.utils import add_all_db, assert_response, create_electrical_cell_recording_id + + +def test_get_electrical_cell_recording( + db, client, create_emodel_ids, electrical_cell_recording_json_data +): + # create two emodels, one with derivations and one without + generated_emodel_id, other_emodel_id = create_emodel_ids(2) + trace_ids = [ + create_electrical_cell_recording_id( + client, json_data=electrical_cell_recording_json_data | {"name": f"name-{i}"} + ) + for i in range(2) + ] + derivations = [ + Derivation(used_id=ecr_id, generated_id=generated_emodel_id) for ecr_id in trace_ids + ] + add_all_db(db, derivations) + + response = client.get(url=f"/emodel/{generated_emodel_id}/generated-by") + + assert_response(response, 200) + data = response.json()["data"] + assert len(data) == 2 + assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} + assert all(d["type"] == "electrical_cell_recording" for d in data) + + response = client.get(url=f"/emodel/{other_emodel_id}/generated-by") + + assert_response(response, 200) + data = response.json()["data"] + assert len(data) == 0 From 4a94eaf6192221946baaeaf217b8336fe6580d12 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Tue, 27 May 2025 16:49:56 +0200 Subject: [PATCH 6/9] Replace generated-by with derived-from --- app/routers/derivation.py | 2 +- tests/test_derivation.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/routers/derivation.py b/app/routers/derivation.py index cb5a96c8..c5be93cc 100644 --- a/app/routers/derivation.py +++ b/app/routers/derivation.py @@ -9,4 +9,4 @@ tags=["derivation"], ) -router.get("/{entity_route}/{entity_id}/generated-by")(app.service.derivation.read_many) +router.get("/{entity_route}/{entity_id}/derived-from")(app.service.derivation.read_many) diff --git a/tests/test_derivation.py b/tests/test_derivation.py index 6d4ee5b9..ab883ad3 100644 --- a/tests/test_derivation.py +++ b/tests/test_derivation.py @@ -19,7 +19,7 @@ def test_get_electrical_cell_recording( ] add_all_db(db, derivations) - response = client.get(url=f"/emodel/{generated_emodel_id}/generated-by") + response = client.get(url=f"/emodel/{generated_emodel_id}/derived-from") assert_response(response, 200) data = response.json()["data"] @@ -27,7 +27,7 @@ def test_get_electrical_cell_recording( assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} assert all(d["type"] == "electrical_cell_recording" for d in data) - response = client.get(url=f"/emodel/{other_emodel_id}/generated-by") + response = client.get(url=f"/emodel/{other_emodel_id}/derived-from") assert_response(response, 200) data = response.json()["data"] From 62076679c9377d04751e32a7388797013a0ba47a Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Fri, 30 May 2025 12:22:56 +0200 Subject: [PATCH 7/9] Remove 1 and 2 1. expose GET /emodel//electrical-cell-recording) 2. allow to search by emodel in GET /electrical-cell-recording?generated_emodel__id= --- app/filters/electrical_cell_recording.py | 16 ++--------- app/queries/factory.py | 4 --- app/routers/emodel.py | 29 -------------------- app/service/electrical_cell_recording.py | 12 +-------- tests/test_derivation.py | 6 ++--- tests/test_electrical_cell_recording.py | 34 ------------------------ tests/test_emodel.py | 23 ---------------- 7 files changed, 5 insertions(+), 119 deletions(-) diff --git a/app/filters/electrical_cell_recording.py b/app/filters/electrical_cell_recording.py index d33d6e6e..bf780317 100644 --- a/app/filters/electrical_cell_recording.py +++ b/app/filters/electrical_cell_recording.py @@ -1,32 +1,20 @@ from typing import Annotated -from fastapi_filter import FilterDepends, with_prefix +from fastapi_filter import FilterDepends -from app.db.model import ElectricalCellRecording, EModel +from app.db.model import ElectricalCellRecording from app.filters.base import CustomFilter from app.filters.common import ( BrainRegionFilterMixin, EntityFilterMixin, - IdFilterMixin, NameFilterMixin, SubjectFilterMixin, ) -class _NestedEModelFilter(IdFilterMixin, CustomFilter): - """Simplified EModel filter allowing to filter only by id and id__in.""" - - class Constants(CustomFilter.Constants): - model = EModel - - class ElectricalCellRecordingFilter( CustomFilter, BrainRegionFilterMixin, SubjectFilterMixin, EntityFilterMixin, NameFilterMixin ): - generated_emodel: Annotated[ - _NestedEModelFilter | None, - FilterDepends(with_prefix("generated_emodel", _NestedEModelFilter)), - ] = None order_by: list[str] = ["-creation_date"] # noqa: RUF012 class Constants(CustomFilter.Constants): diff --git a/app/queries/factory.py b/app/queries/factory.py index 0a45b946..8ac4bbd8 100644 --- a/app/queries/factory.py +++ b/app/queries/factory.py @@ -4,7 +4,6 @@ Agent, BrainRegion, Contribution, - Derivation, EModel, ETypeClass, ETypeClassification, @@ -107,9 +106,6 @@ def _get_alias[T: type[Identifiable]](db_cls: T, name: str | None = None) -> T: morphology_alias, db_model_class.exemplar_morphology_id == morphology_alias.id ), "emodel": lambda q: q.join(emodel_alias, db_model_class.emodel_id == emodel_alias.id), - "generated_emodel": lambda q: q.join( - Derivation, Derivation.used_id == db_model_class.id - ).join(emodel_alias, Derivation.generated_id == emodel_alias.id), "me_model": lambda q: q.join( me_model_alias, db_model_class.me_model_id == me_model_alias.id ), diff --git a/app/routers/emodel.py b/app/routers/emodel.py index d4b764d2..ffb9edbd 100644 --- a/app/routers/emodel.py +++ b/app/routers/emodel.py @@ -1,15 +1,7 @@ -import uuid - from fastapi import APIRouter import app.service.electrical_cell_recording import app.service.emodel -from app.dependencies.auth import UserContextDep -from app.dependencies.common import InBrainRegionQuery, PaginationQuery, Search, WithFacets -from app.dependencies.db import SessionDep -from app.filters.electrical_cell_recording import ElectricalCellRecordingFilter -from app.schemas.electrical_cell_recording import ElectricalCellRecordingRead -from app.schemas.types import ListResponse router = APIRouter( prefix="/emodel", @@ -19,24 +11,3 @@ read_many = router.get("")(app.service.emodel.read_many) read_one = router.get("/{id_}")(app.service.emodel.read_one) create_one = router.post("")(app.service.emodel.create_one) - - -@router.get("/{id_}/electrical-cell-recording") -def get_electrical_cell_recording( - *, - user_context: UserContextDep, - db: SessionDep, - id_: uuid.UUID, - pagination_request: PaginationQuery, -) -> ListResponse[ElectricalCellRecordingRead]: - """Return the list of ElectricalCellRecording used to generate the specified EModel.""" - filter_model = ElectricalCellRecordingFilter.model_validate({"generated_emodel": {"id": id_}}) - return app.service.electrical_cell_recording.read_many( - user_context=user_context, - db=db, - pagination_request=pagination_request, - filter_model=filter_model, - with_search=Search(), - facets=WithFacets(), - in_brain_region=InBrainRegionQuery(), - ) diff --git a/app/service/electrical_cell_recording.py b/app/service/electrical_cell_recording.py index ad7d492f..dd9d5056 100644 --- a/app/service/electrical_cell_recording.py +++ b/app/service/electrical_cell_recording.py @@ -7,7 +7,6 @@ from app.db.model import ( Agent, ElectricalCellRecording, - EModel, Subject, ) from app.dependencies.auth import UserContextDep, UserContextWithProjectIdDep @@ -86,28 +85,19 @@ def read_many( agent_alias = aliased(Agent, flat=True) created_by_alias = aliased(Agent, flat=True) updated_by_alias = aliased(Agent, flat=True) - emodel_alias = aliased(EModel, flat=True) aliases: Aliases = { Agent: { "contribution": agent_alias, "created_by": created_by_alias, "updated_by": updated_by_alias, }, - EModel: emodel_alias, } - facet_keys = [ + facet_keys = filter_keys = [ "brain_region", "created_by", "updated_by", "contribution", ] - filter_keys = [ - "brain_region", - "created_by", - "updated_by", - "contribution", - "generated_emodel", - ] name_to_facet_query_params, filter_joins = query_params_factory( db_model_class=ElectricalCellRecording, facet_keys=facet_keys, diff --git a/tests/test_derivation.py b/tests/test_derivation.py index ab883ad3..62f3557a 100644 --- a/tests/test_derivation.py +++ b/tests/test_derivation.py @@ -3,9 +3,7 @@ from tests.utils import add_all_db, assert_response, create_electrical_cell_recording_id -def test_get_electrical_cell_recording( - db, client, create_emodel_ids, electrical_cell_recording_json_data -): +def test_get_derived_from(db, client, create_emodel_ids, electrical_cell_recording_json_data): # create two emodels, one with derivations and one without generated_emodel_id, other_emodel_id = create_emodel_ids(2) trace_ids = [ @@ -24,7 +22,7 @@ def test_get_electrical_cell_recording( assert_response(response, 200) data = response.json()["data"] assert len(data) == 2 - assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} + assert [data[0]["id"], data[1]["id"]] == [str(id_) for id_ in reversed(trace_ids)] assert all(d["type"] == "electrical_cell_recording" for d in data) response = client.get(url=f"/emodel/{other_emodel_id}/derived-from") diff --git a/tests/test_electrical_cell_recording.py b/tests/test_electrical_cell_recording.py index 952a2509..1ded43a1 100644 --- a/tests/test_electrical_cell_recording.py +++ b/tests/test_electrical_cell_recording.py @@ -2,14 +2,11 @@ import pytest -from app.db.model import Derivation from app.db.types import EntityType from .utils import ( PROJECT_ID, - add_all_db, assert_request, - assert_response, check_authorization, check_brain_region_filter, check_missing, @@ -171,34 +168,3 @@ def create_model_function(_db, name, brain_region_id): ) check_brain_region_filter(ROUTE, client, db, brain_region_hierarchy_id, create_model_function) - - -def test_get_traces_for_generated_emodel( - db, client, create_emodel_ids, electrical_cell_recording_json_data -): - # create two emodels, one with derivations and one without - generated_emodel_id, other_emodel_id = create_emodel_ids(2) - trace_ids = [ - create_electrical_cell_recording_id( - client, json_data=electrical_cell_recording_json_data | {"name": f"name-{i}"} - ) - for i in range(2) - ] - derivations = [ - Derivation(used_id=ecr_id, generated_id=generated_emodel_id) for ecr_id in trace_ids - ] - add_all_db(db, derivations) - - response = client.get(url=ROUTE, params={"generated_emodel__id": generated_emodel_id}) - - assert_response(response, 200) - data = response.json()["data"] - assert len(data) == 2 - assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} - assert all(d["type"] == "electrical_cell_recording" for d in data) - - response = client.get(url=ROUTE, params={"generated_emodel__id": other_emodel_id}) - - assert_response(response, 200) - data = response.json()["data"] - assert len(data) == 0 diff --git a/tests/test_emodel.py b/tests/test_emodel.py index d286e2f9..c04b785b 100644 --- a/tests/test_emodel.py +++ b/tests/test_emodel.py @@ -3,14 +3,10 @@ from fastapi.testclient import TestClient -from app.db.model import Derivation from app.db.types import EntityType from .conftest import CreateIds, EModelIds from .utils import ( - add_all_db, - assert_response, - create_electrical_cell_recording_id, create_reconstruction_morphology_id, ) from tests.routers.test_asset import _upload_entity_asset @@ -372,22 +368,3 @@ def test_pagination(client, create_emodel_ids): assert len(items) == total_items data_ids = [int(i["name"]) for i in items] assert list(reversed(data_ids)) == list(range(total_items)) - - -def test_get_electrical_cell_recording(client, db, emodel_id, electrical_cell_recording_json_data): - trace_ids = [ - create_electrical_cell_recording_id( - client, json_data=electrical_cell_recording_json_data | {"name": f"name-{i}"} - ) - for i in range(2) - ] - derivations = [Derivation(used_id=ecr_id, generated_id=emodel_id) for ecr_id in trace_ids] - add_all_db(db, derivations) - - response = client.get(f"{ROUTE}/{emodel_id}/electrical-cell-recording") - - assert_response(response, 200) - data = response.json()["data"] - assert len(data) == 2 - assert {data[0]["id"], data[1]["id"]} == {str(id_) for id_ in trace_ids} - assert all(d["type"] == "electrical_cell_recording" for d in data) From 5be625fc68cac4e04b46ebace2733fd2a583b193 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Mon, 2 Jun 2025 14:52:26 +0200 Subject: [PATCH 8/9] Update migration after merge --- ...2_145126_8f0d631d2bd4_default_migration_message.py} | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) rename alembic/versions/{20250527_114536_d20b8917d3ab_default_migration_message.py => 20250602_145126_8f0d631d2bd4_default_migration_message.py} (86%) diff --git a/alembic/versions/20250527_114536_d20b8917d3ab_default_migration_message.py b/alembic/versions/20250602_145126_8f0d631d2bd4_default_migration_message.py similarity index 86% rename from alembic/versions/20250527_114536_d20b8917d3ab_default_migration_message.py rename to alembic/versions/20250602_145126_8f0d631d2bd4_default_migration_message.py index 2404815d..0d5b61ef 100644 --- a/alembic/versions/20250527_114536_d20b8917d3ab_default_migration_message.py +++ b/alembic/versions/20250602_145126_8f0d631d2bd4_default_migration_message.py @@ -1,8 +1,8 @@ """Default migration message -Revision ID: d20b8917d3ab -Revises: 8d1610d7c882 -Create Date: 2025-05-27 11:45:36.409166 +Revision ID: 8f0d631d2bd4 +Revises: 1589bff44728 +Create Date: 2025-06-02 14:51:26.859717 """ @@ -16,8 +16,8 @@ import app.db.types # revision identifiers, used by Alembic. -revision: str = "d20b8917d3ab" -down_revision: Union[str, None] = "8d1610d7c882" +revision: str = "8f0d631d2bd4" +down_revision: Union[str, None] = "1589bff44728" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None From 98bd2f5e15efe1044a38fe72f53cfb764a713327 Mon Sep 17 00:00:00 2001 From: Gianluca Ficarelli Date: Mon, 2 Jun 2025 15:39:40 +0200 Subject: [PATCH 9/9] Remove 3 --- app/db/model.py | 6 ------ app/schemas/emodel.py | 2 -- app/service/emodel.py | 1 - tests/test_emodel.py | 1 - 4 files changed, 10 deletions(-) diff --git a/app/db/model.py b/app/db/model.py index e60a4489..02fd425f 100644 --- a/app/db/model.py +++ b/app/db/model.py @@ -459,12 +459,6 @@ class EModel( viewonly=True, order_by="IonChannelModel.creation_date", ) - electrical_cell_recordings: Mapped[list["ElectricalCellRecording"]] = relationship( - primaryjoin="EModel.id == Derivation.generated_id", - secondaryjoin="Entity.id == Derivation.used_id", - secondary="derivation", - viewonly=True, - ) __mapper_args__ = {"polymorphic_identity": __tablename__} # noqa: RUF012 diff --git a/app/schemas/emodel.py b/app/schemas/emodel.py index ff989c78..580998b9 100644 --- a/app/schemas/emodel.py +++ b/app/schemas/emodel.py @@ -8,7 +8,6 @@ from app.schemas.base import ( AuthorizationMixin, AuthorizationOptionalPublicMixin, - BasicEntityRead, BrainRegionRead, CreationMixin, EntityTypeMixin, @@ -61,4 +60,3 @@ class EModelRead( class EModelReadExpanded(EModelRead, AssetsMixin): ion_channel_models: list[IonChannelModelWAssets] - electrical_cell_recordings: list[BasicEntityRead] diff --git a/app/service/emodel.py b/app/service/emodel.py index 5bfa9f2c..1767f2c0 100644 --- a/app/service/emodel.py +++ b/app/service/emodel.py @@ -44,7 +44,6 @@ def _load(select: sa.Select[tuple[EModel]]): selectinload(EModel.ion_channel_models).joinedload(IonChannelModel.strain), selectinload(EModel.ion_channel_models).joinedload(IonChannelModel.brain_region), selectinload(EModel.ion_channel_models).selectinload(IonChannelModel.assets), - selectinload(EModel.electrical_cell_recordings), joinedload(EModel.created_by), joinedload(EModel.updated_by), raiseload("*"), diff --git a/tests/test_emodel.py b/tests/test_emodel.py index fa537273..472c2ef1 100644 --- a/tests/test_emodel.py +++ b/tests/test_emodel.py @@ -59,7 +59,6 @@ def test_get_emodel(client: TestClient, emodel_id: str): assert "assets" in data assert len(data["assets"]) == 1 assert "ion_channel_models" in data - assert "electrical_cell_recordings" in data assert data["created_by"]["id"] == data["updated_by"]["id"]