From d7748c060360d738380d9150d966546b826881e3 Mon Sep 17 00:00:00 2001 From: Jan-Lukas Wynen Date: Tue, 14 Oct 2025 17:40:01 +0200 Subject: [PATCH] Add support for proposals --- justfile | 4 + src/scitacean/client.py | 100 +++++++++++ src/scitacean/model.py | 210 ++++++++++++++++++++++++ src/scitacean/testing/client.py | 45 +++++ tests/client/proposal_client_test.py | 113 +++++++++++++ tests/client/sample_client_test.py | 1 + tools/model-generation/spec/__init__.py | 4 +- 7 files changed, 476 insertions(+), 1 deletion(-) create mode 100644 tests/client/proposal_client_test.py diff --git a/justfile b/justfile index 16673231..b900be46 100644 --- a/justfile +++ b/justfile @@ -75,6 +75,10 @@ clean-docs: build: @uv run --group=build python -m build +[working-directory: 'tools/model-generation'] +generate-models: + @uv run python generate_models.py --launch-scicat + # Remove the output from a Jupyter notebook strip-output *notebooks: @uv run --group=format nbstripout \ diff --git a/src/scitacean/client.py b/src/scitacean/client.py index 3930054a..251d9d39 100644 --- a/src/scitacean/client.py +++ b/src/scitacean/client.py @@ -247,6 +247,33 @@ def get_dataset( attachment_models=attachment_models, ) + def get_proposal( + self, + proposal_id: str, + strict_validation: bool = False, + ) -> model.Proposal: + """Download a proposal from SciCat. + + Parameters + ---------- + proposal_id: + ID of the proposal. + strict_validation: + If ``True``, the sample must pass validation. + If ``False``, a sample is still returned if validation fails. + Note that some sample fields may have a bad value or type. + A warning will be logged if validation fails. + + Returns + ------- + : + The downloaded proposal. + """ + proposal_model = self.scicat.get_proposal_model( + proposal_id, strict_validation=strict_validation + ) + return model.Proposal.from_download_model(proposal_model) + def get_sample( self, sample_id: str, @@ -929,6 +956,47 @@ def get_attachments_for_dataset( for attachment in attachment_json ] + def get_proposal_model( + self, proposal_id: str, strict_validation: bool = False + ) -> model.DownloadProposal: + """Fetch a proposal from SciCat. + + Parameters + ---------- + proposal_id: + ID of the proposal to fetch. + strict_validation: + If ``True``, the sample must pass validation. + If ``False``, a sample is still returned if validation fails. + Note that some fields may have a bad value or type. + A warning will be logged if validation fails. + + Returns + ------- + : + A model of the proposal. + + Raises + ------ + scitacean.ScicatCommError + If the proposal does not exist or communication fails for some other reason. + """ + proposal_json = self._call_endpoint( + cmd="get", + url=f"proposals/{quote_plus(proposal_id)}", + operation="get_proposal_model", + ) + if not proposal_json: + raise ScicatCommError( + f"Cannot get proposal with {proposal_id=}, " + f"no such proposal in SciCat at {self._base_url}." + ) + return model.construct( + model.DownloadProposal, + _strict_validation=strict_validation, + **proposal_json, + ) + def get_sample_model( self, sample_id: str, strict_validation: bool = False ) -> model.DownloadSample: @@ -1093,6 +1161,38 @@ def create_attachment_for_dataset( model.DownloadAttachment, _strict_validation=False, **uploaded ) + def create_proposal_model( + self, proposal: model.UploadProposal + ) -> model.DownloadProposal: + """Create a new proposal in SciCat. + + Parameters + ---------- + proposal: + Model of the proposal to create. + + Returns + ------- + : + The uploaded proposal as returned by SciCat. + This is the input plus some modifications. + + Raises + ------ + scitacean.ScicatCommError + If SciCat refuses the proposal or communication + fails for some other reason. + """ + uploaded = self._call_endpoint( + cmd="post", + url="proposals", + data=proposal, + operation="create_proposal_model", + ) + return model.construct( + model.DownloadProposal, _strict_validation=False, **uploaded + ) + def create_sample_model(self, sample: model.UploadSample) -> model.DownloadSample: """Create a new sample in SciCat. diff --git a/src/scitacean/model.py b/src/scitacean/model.py index 7045a6f1..d873f245 100644 --- a/src/scitacean/model.py +++ b/src/scitacean/model.py @@ -30,6 +30,8 @@ History Instrument Lifecycle + MeasurementPeriod + Proposal Relationship Sample Technique @@ -48,7 +50,9 @@ DownloadHistory DownloadInstrument DownloadLifecycle + DownloadMeasurementPeriod DownloadOrigDatablock + DownloadProposal DownloadRelationship DownloadSample DownloadTechnique @@ -64,7 +68,9 @@ UploadDatablock UploadDataFile UploadDerivedDataset + UploadMeasurementPeriod UploadOrigDatablock + UploadProposal UploadRawDataset UploadRelationship UploadSample @@ -80,6 +86,7 @@ from __future__ import annotations +import builtins from dataclasses import dataclass from datetime import datetime from typing import Any @@ -555,6 +562,78 @@ def user_model_type(cls) -> type[Instrument]: return Instrument +class DownloadProposal(BaseModel): + email: str | None = None + ownerGroup: str | None = None + proposalId: str | None = None + title: str | None = None + measurementPeriodList: list[DownloadMeasurementPeriod] | None = None + abstract: str | None = None + accessGroups: list[str] | None = None + createdAt: datetime | None = None + createdBy: str | None = None + endTime: datetime | None = None + firstname: str | None = None + instrumentGroup: str | None = None + isPublished: bool | None = None + lastname: str | None = None + metadata: dict[str, Any] | None = None + parentProposalId: str | None = None + pi_email: str | None = None + pi_firstname: str | None = None + pi_lastname: str | None = None + startTime: datetime | None = None + type: str | None = None + updatedAt: datetime | None = None + updatedBy: str | None = None + + @classmethod + def user_model_type(cls) -> builtins.type[Proposal]: + return Proposal + + # Custom properties to work around naming inconsistencies in SciCat. + @property + def piEmail(self) -> str | None: + return self.pi_email + + @property + def piFirstname(self) -> str | None: + return self.pi_firstname + + @property + def piLastname(self) -> str | None: + return self.pi_lastname + + +class UploadProposal(BaseModel): + email: str + ownerGroup: str + proposalId: str + title: str + MeasurementPeriodList: list[UploadMeasurementPeriod] | None = None + abstract: str | None = None + accessGroups: list[str] | None = None + endTime: datetime | None = None + firstname: str | None = None + instrumentGroup: str | None = None + lastname: str | None = None + metadata: dict[str, Any] | None = None + parentProposalId: str | None = None + pi_email: str | None = None + pi_firstname: str | None = None + pi_lastname: str | None = None + startTime: datetime | None = None + type: str | None = None + + @classmethod + def user_model_type(cls) -> builtins.type[Proposal]: + return Proposal + + @classmethod + def download_model_type(cls) -> builtins.type[DownloadProposal]: + return DownloadProposal + + class DownloadSample(BaseModel): ownerGroup: str | None = None accessGroups: list[str] | None = None @@ -601,6 +680,36 @@ def download_model_type(cls) -> type[DownloadSample]: return DownloadSample +class DownloadMeasurementPeriod(BaseModel): + comment: str | None = None + end: datetime | None = None + instrument: str | None = None + start: datetime | None = None + + @classmethod + def user_model_type(cls) -> type[MeasurementPeriod]: + return MeasurementPeriod + + @classmethod + def upload_model_type(cls) -> type[UploadMeasurementPeriod]: + return UploadMeasurementPeriod + + +class UploadMeasurementPeriod(BaseModel): + end: datetime + instrument: str + start: datetime + comment: str | None = None + + @classmethod + def user_model_type(cls) -> type[MeasurementPeriod]: + return MeasurementPeriod + + @classmethod + def download_model_type(cls) -> type[DownloadMeasurementPeriod]: + return DownloadMeasurementPeriod + + @dataclass(kw_only=True, slots=True) class Attachment(BaseUserModel): caption: str @@ -846,6 +955,70 @@ def download_model_type(cls) -> type[DownloadInstrument]: return DownloadInstrument +@dataclass(kw_only=True, slots=True) +class Proposal(BaseUserModel): + email: str + owner_group: str + proposal_id: str + title: str + measurement_period_list: list[MeasurementPeriod] | None = None + abstract: str | None = None + access_groups: list[str] | None = None + end_time: datetime | None = None + firstname: str | None = None + instrument_group: str | None = None + lastname: str | None = None + metadata: dict[str, Any] | None = None + parent_proposal_id: str | None = None + pi_email: str | None = None + pi_firstname: str | None = None + pi_lastname: str | None = None + start_time: datetime | None = None + type: str | None = None + _created_at: datetime | None = None + _created_by: str | None = None + _is_published: bool | None = None + _updated_at: datetime | None = None + _updated_by: str | None = None + + @property + def created_at(self) -> datetime | None: + return self._created_at + + @property + def created_by(self) -> str | None: + return self._created_by + + @property + def is_published(self) -> bool | None: + return self._is_published + + @property + def updated_at(self) -> datetime | None: + return self._updated_at + + @property + def updated_by(self) -> str | None: + return self._updated_by + + @classmethod + def from_download_model(cls, download_model: DownloadProposal) -> Proposal: + """Construct an instance from an associated SciCat download model.""" + return cls(**cls._download_model_dict(download_model)) + + def make_upload_model(self) -> UploadProposal: + """Construct a SciCat upload model from self.""" + return UploadProposal(**self._upload_model_dict()) + + @classmethod + def upload_model_type(cls) -> builtins.type[UploadProposal]: + return UploadProposal + + @classmethod + def download_model_type(cls) -> builtins.type[DownloadProposal]: + return DownloadProposal + + @dataclass(kw_only=True, slots=True) class Sample(BaseUserModel): owner_group: str @@ -895,6 +1068,33 @@ def download_model_type(cls) -> type[DownloadSample]: return DownloadSample +@dataclass(kw_only=True, slots=True) +class MeasurementPeriod(BaseUserModel): + end: datetime + instrument: str + start: datetime + comment: str | None = None + + @classmethod + def from_download_model( + cls, download_model: DownloadMeasurementPeriod + ) -> MeasurementPeriod: + """Construct an instance from an associated SciCat download model.""" + return cls(**cls._download_model_dict(download_model)) + + def make_upload_model(self) -> UploadMeasurementPeriod: + """Construct a SciCat upload model from self.""" + return UploadMeasurementPeriod(**self._upload_model_dict()) + + @classmethod + def upload_model_type(cls) -> type[UploadMeasurementPeriod]: + return UploadMeasurementPeriod + + @classmethod + def download_model_type(cls) -> type[DownloadMeasurementPeriod]: + return DownloadMeasurementPeriod + + # Some models contain fields that are other models which are defined # further down in the file. # Instead of ordering models according to their dependencies, resolve @@ -914,8 +1114,12 @@ def download_model_type(cls) -> type[DownloadSample]: DownloadDataFile.model_rebuild() UploadDataFile.model_rebuild() DownloadInstrument.model_rebuild() +DownloadProposal.model_rebuild() +UploadProposal.model_rebuild() DownloadSample.model_rebuild() UploadSample.model_rebuild() +DownloadMeasurementPeriod.model_rebuild() +UploadMeasurementPeriod.model_rebuild() DownloadDataset.model_rebuild() UploadDerivedDataset.model_rebuild() UploadRawDataset.model_rebuild() @@ -932,13 +1136,17 @@ def download_model_type(cls) -> type[DownloadSample]: "DownloadHistory", "DownloadInstrument", "DownloadLifecycle", + "DownloadMeasurementPeriod", "DownloadOrigDatablock", + "DownloadProposal", "DownloadRelationship", "DownloadSample", "DownloadTechnique", "History", "Instrument", "Lifecycle", + "MeasurementPeriod", + "Proposal", "Relationship", "Sample", "Technique", @@ -946,7 +1154,9 @@ def download_model_type(cls) -> type[DownloadSample]: "UploadDataFile", "UploadDatablock", "UploadDerivedDataset", + "UploadMeasurementPeriod", "UploadOrigDatablock", + "UploadProposal", "UploadRawDataset", "UploadRelationship", "UploadSample", diff --git a/src/scitacean/testing/client.py b/src/scitacean/testing/client.py index 4186d202..e453cd7c 100644 --- a/src/scitacean/testing/client.py +++ b/src/scitacean/testing/client.py @@ -129,6 +129,7 @@ def __init__( self.datasets: dict[PID, model.DownloadDataset] = {} self.orig_datablocks: dict[PID, list[model.DownloadOrigDatablock]] = {} self.attachments: dict[PID, list[model.DownloadAttachment]] = {} + self.proposals: dict[str, model.DownloadProposal] = {} self.samples: dict[str, model.DownloadSample] = {} @classmethod @@ -225,6 +226,19 @@ def get_attachments_for_dataset( _ = strict_validation # unused by fake return self.main.attachments.get(pid) or [] + @_conditionally_disabled + def get_proposal_model( + self, proposal_id: str, strict_validation: bool = False + ) -> model.DownloadProposal: + """Fetch a proposal from SciCat.""" + _ = strict_validation # unused by fake + try: + return self.main.proposals[proposal_id] + except KeyError: + raise ScicatCommError( + f"Unable to retrieve proposal {proposal_id}" + ) from None + @_conditionally_disabled def get_sample_model( self, sample_id: str, strict_validation: bool = False @@ -283,6 +297,21 @@ def create_attachment_for_dataset( self.main.attachments.setdefault(dataset_id, []).append(ingested) return ingested + @_conditionally_disabled + def create_proposal_model( + self, proposal: model.UploadProposal + ) -> model.DownloadProposal: + """Create a new proposal in SciCat.""" + ingested = _process_proposal(proposal) + proposal_id: str = ingested.proposalId # type: ignore[assignment] + if proposal_id in self.main.proposals: + raise ScicatCommError( + f"Cannot create proposal with id '{proposal_id}' " + "because there already is a proposal with this id." + ) + self.main.proposals[proposal_id] = ingested + return ingested + @_conditionally_disabled def create_sample_model(self, sample: model.UploadSample) -> model.DownloadSample: """Create a new sample in SciCat.""" @@ -408,6 +437,22 @@ def _process_attachment( ) +def _process_proposal(proposal: model.UploadProposal) -> model.DownloadProposal: + created_at = datetime.datetime.now(tz=datetime.UTC) + fields = _model_dict(proposal) + # Using strict_validation=False because the input model should already be validated. + # If there are validation errors, it was probably intended by the user. + return model.construct( + model.DownloadProposal, + _strict_validation=False, + createdBy="fake", + createdAt=created_at, + updatedBy="fake", + updatedAt=created_at, + **fields, + ) + + def _process_sample(sample: model.UploadSample) -> model.DownloadSample: created_at = datetime.datetime.now(tz=datetime.UTC) fields = _model_dict(sample) diff --git a/tests/client/proposal_client_test.py b/tests/client/proposal_client_test.py new file mode 100644 index 00000000..f5f5ac71 --- /dev/null +++ b/tests/client/proposal_client_test.py @@ -0,0 +1,113 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2025 SciCat Project (https://github.com/SciCatProject/scitacean) + +import dataclasses +from datetime import UTC, datetime + +import pytest + +from scitacean import Client, ScicatCommError +from scitacean.client import ScicatClient +from scitacean.model import ( + DownloadProposal, + MeasurementPeriod, + Proposal, + UploadProposal, +) +from scitacean.testing.backend import config as backend_config + + +# Creating proposals requires at least ingestor permissions. +@pytest.fixture +def ingestor_access( + scicat_access: backend_config.SciCatAccess, +) -> backend_config.SciCatAccess: + return backend_config.SciCatAccess( + url=scicat_access.url, + user=backend_config.USERS["ingestor"], + ) + + +# Override the default real_client fixture to use ingestor permissions. +@pytest.fixture +def real_client( + request: pytest.FixtureRequest, + ingestor_access: backend_config.SciCatAccess, + scicat_backend: bool, +) -> Client: + pytest.skip("No available user can create proposals.") + # skip_if_not_backend(request) + # return Client.from_credentials( + # url=ingestor_access.url, + # **ingestor_access.user.credentials, + # ) + + +@pytest.fixture +def scicat_client(client: Client) -> ScicatClient: + return client.scicat + + +@pytest.fixture +def proposal(ingestor_access: backend_config.SciCatAccess) -> Proposal: + scicat_access = ingestor_access + return Proposal( + email=scicat_access.user.email, + owner_group=scicat_access.user.group, + proposal_id="123def", + title="A test proposal", + access_groups=["group1", "2nd_group"], + pi_email="principal@investigator.net", + pi_firstname="John", + pi_lastname="Principal", + measurement_period_list=[ + MeasurementPeriod( + start=datetime(2022, 11, 16, 15, 6, 2, tzinfo=UTC), + end=datetime(2022, 11, 17, 8, 0, 50, tzinfo=UTC), + instrument="Neutrometer", + ) + ], + ) + + +def compare_proposal_model_after_upload( + uploaded: UploadProposal | DownloadProposal, downloaded: DownloadProposal +) -> None: + for key, expected in uploaded: + # The database populates a number of fields that are None in uploaded. + # But we don't want to test those here as we don't want to test the database. + if expected is not None: + assert expected == dict(downloaded)[key], f"key = {key}" + + +def compare_proposal_after_upload(uploaded: Proposal, downloaded: Proposal) -> None: + for field in dataclasses.fields(uploaded): + # The database populates a number of fields that are None in uploaded. + # But we don't want to test those here as we don't want to test the database. + expected = getattr(uploaded, field.name) + if expected is not None: + assert expected == getattr(downloaded, field.name), f"key = {field.name}" + + +def test_create_proposal_model_roundtrip( + scicat_client: ScicatClient, proposal: Proposal +) -> None: + upload_proposal = proposal.make_upload_model() + finalized = scicat_client.create_proposal_model(upload_proposal) + assert finalized.proposalId == proposal.proposal_id + downloaded = scicat_client.get_proposal_model(finalized.proposalId) + compare_proposal_model_after_upload(upload_proposal, downloaded) + compare_proposal_model_after_upload(finalized, downloaded) + + +def test_create_sample_model_roundtrip_existing_id( + scicat_client: ScicatClient, proposal: Proposal +) -> None: + upload_proposal = proposal.make_upload_model() + finalized = scicat_client.create_proposal_model(upload_proposal) + assert finalized.proposalId is not None + upload_proposal.proposalId = finalized.proposalId + # SciCat might accept an identical proposal, so change it. + upload_proposal.abstract = "A new proposal with the same id" + with pytest.raises(ScicatCommError): + scicat_client.create_proposal_model(upload_proposal) diff --git a/tests/client/sample_client_test.py b/tests/client/sample_client_test.py index 1c692221..ff7f2563 100644 --- a/tests/client/sample_client_test.py +++ b/tests/client/sample_client_test.py @@ -23,6 +23,7 @@ def ingestor_access( ) +# Override the default real_client fixture to use ingestor permissions. @pytest.fixture def real_client( request: pytest.FixtureRequest, diff --git a/tools/model-generation/spec/__init__.py b/tools/model-generation/spec/__init__.py index f815db07..9caa4f13 100644 --- a/tools/model-generation/spec/__init__.py +++ b/tools/model-generation/spec/__init__.py @@ -143,7 +143,7 @@ def user_dset_fields(self, manual: bool | None = None) -> list[DatasetField]: _SCHEMA_GROUPS = { - "Attachment": ("CreateAttachmentDto", "Attachment"), + "Attachment": ("CreateAttachmentV3Dto", "Attachment"), "OrigDatablock": ("CreateDatasetOrigDatablockDto", "OrigDatablock"), "Datablock": ("CreateDatasetDatablockDto", "Datablock"), "Lifecycle": (None, "LifecycleClass"), @@ -152,7 +152,9 @@ def user_dset_fields(self, manual: bool | None = None) -> list[DatasetField]: "History": (None, "HistoryClass"), "DataFile": ("DataFile", "DataFile"), "Instrument": (None, "Instrument"), + "Proposal": ("CreateProposalDto", "ProposalClass"), "Sample": ("CreateSampleDto", "SampleClass"), + "MeasurementPeriod": ("CreateMeasurementPeriodDto", "MeasurementPeriodClass"), }