From b220051c6430b2eca1e2d0355d81670f3605fd8a Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 12:27:57 +0100 Subject: [PATCH 1/9] Implement upload functions for proposals, samples and instruments --- pyscicat/client.py | 97 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/pyscicat/client.py b/pyscicat/client.py index d0b8a0f..4d9fbc7 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -16,9 +16,12 @@ Attachment, Datablock, Dataset, + DerivedDataset, + Instrument, OrigDatablock, + Proposal, RawDataset, - DerivedDataset, + Sample, ) logger = logging.getLogger("splash_ingest") @@ -413,6 +416,98 @@ def datasets_attachment_create( upload_attachment = datasets_attachment_create create_dataset_attachment = datasets_attachment_create + def samples_create(self, sample: Sample) -> str: + """ + Create a new sample or update an existing one. + This function is also accessible as upload_sample. + + + Parameters + ---------- + sample : Sample + Sample to upload + + Returns + ------- + str + id of the newly created sample + + Raises + ------ + ScicatCommError + Raises if a non-20x message is returned + """ + return self._call_endpoint( + cmd="post", + endpoint="Samples", + data=sample, + operation="samples_create", + ).get("sampleId") + + upload_sample = samples_create + + def instruments_create(self, instrument: Instrument): + """ + Create a new instrument or update an existing one. + Note that in SciCat admin rights are required to upload instruments. + This function is also accessible as upload_instrument. + + + Parameters + ---------- + instrument : Instrument + Instrument to upload + + Returns + ------- + str + pid (or unique identifier) of the newly created instrument + + Raises + ------ + ScicatCommError + Raises if a non-20x message is returned + """ + return self._call_endpoint( + cmd="post", + endpoint="Instruments", + data=instrument, + operation="instruments_create", + ).get("pid") + + upload_instrument = instruments_create + + def proposals_create(self, proposal: Proposal): + """ + Create a new proposal or update an existing one. + Note that in SciCat admin rights are required to upload proposals. + This function is also accessible as upload_proposal. + + + Parameters + ---------- + proposal : Proposal + Proposal to upload + + Returns + ------- + str + id of the newly created proposal + + Raises + ------ + ScicatCommError + Raises if a non-20x message is returned + """ + return self._call_endpoint( + cmd="post", + endpoint="Proposals", + data=proposal, + operation="proposals_create", + ).get("proposalId") + + upload_proposal = proposals_create + def datasets_find( self, skip: int = 0, limit: int = 25, query_fields: Optional[dict] = None ) -> Optional[dict]: From 3f8cd33a6032edd58ae9311d55df07d05d0aba6a Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 12:58:06 +0100 Subject: [PATCH 2/9] Test upload functions for proposals, samples and instruments --- pyscicat/tests/test_client.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/pyscicat/tests/test_client.py b/pyscicat/tests/test_client.py index c9069fc..685033b 100644 --- a/pyscicat/tests/test_client.py +++ b/pyscicat/tests/test_client.py @@ -16,7 +16,10 @@ Attachment, Datablock, DataFile, + Instrument, + Proposal, RawDataset, + Sample, Ownable, ) @@ -28,7 +31,11 @@ def add_mock_requests(mock_request): local_url + "Users/login", json={"id": "a_token"}, ) - mock_request.post(local_url + "Samples", json={"sampleId": "dataset_id"}) + + mock_request.post(local_url + "Instruments", json={"pid": "earth"}) + mock_request.post(local_url + "Proposals", json={"proposalId": "deepthought"}) + mock_request.post(local_url + "Samples", json={"sampleId": "gargleblaster"}) + mock_request.post(local_url + "RawDatasets/replaceOrCreate", json={"pid": "42"}) mock_request.patch( local_url + "Datasets/42", @@ -66,6 +73,30 @@ def test_scicat_ingest(): size = get_file_size(thumb_path) assert size is not None + # Instrument + instrument = Instrument( + pid="earth", name="Earth", customMetadata={"a": "field"} + ) + assert scicat.upload_instrument(instrument) == "earth" + assert scicat.instruments_create(instrument) == "earth" + + # Proposal + proposal = Proposal( + proposalId="deepthought", title="Deepthought", **ownable.dict() + ) + assert scicat.upload_proposal(proposal) == "deepthought" + assert scicat.proposals_create(proposal) == "deepthought" + + # Sample + sample = Sample( + sampleId="gargleblaster", + description="Gargleblaster", + sampleCharacteristics={"a": "field"}, + **ownable.dict() + ) + assert scicat.upload_sample(sample) == "gargleblaster" + assert scicat.samples_create(proposal) == "gargleblaster" + # RawDataset dataset = RawDataset( path="/foo/bar", From 6216e100360529f1e6ce7975222d0afaee2c282d Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 14:39:50 +0100 Subject: [PATCH 3/9] Update data model --- pyscicat/model.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pyscicat/model.py b/pyscicat/model.py index bcaab4b..837e9bf 100644 --- a/pyscicat/model.py +++ b/pyscicat/model.py @@ -26,7 +26,8 @@ class Ownable(MongoQueryable): """Many objects in SciCat are ownable""" ownerGroup: str - accessGroups: List[str] + accessGroups: Optional[List[str]] + instrumentGroup: Optional[str] class User(BaseModel): @@ -45,15 +46,14 @@ class Proposal(Ownable): Defines the purpose of an experiment and links an experiment to principal investigator and main proposer """ - # TODO: find out which of these are not optional and update - proposalId: Optional[str] + proposalId: str pi_email: Optional[str] pi_firstname: Optional[str] pi_lastname: Optional[str] - email: Optional[str] + email: str firstname: Optional[str] lastname: Optional[str] - title: Optional[str] + title: Optional[str] # required in next backend version abstract: Optional[str] startTime: Optional[str] endTime: Optional[str] @@ -68,7 +68,6 @@ class Sample(Ownable): Raw datasets should be linked to such sample definitions. """ - # TODO: find out which of these are not optional and update sampleId: Optional[str] owner: Optional[str] description: Optional[str] From 15364eff9a898e11ae9f9a5b3715c40e6ec31fd3 Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 16:28:23 +0100 Subject: [PATCH 4/9] Update documentation --- docs/source/howto/ingest.md | 14 ++++++++++++++ pyscicat/client.py | 3 +++ 2 files changed, 17 insertions(+) diff --git a/docs/source/howto/ingest.md b/docs/source/howto/ingest.md index 05cf37d..a694ed0 100644 --- a/docs/source/howto/ingest.md +++ b/docs/source/howto/ingest.md @@ -13,6 +13,7 @@ from pyscicat.model import ( Datablock, DataFile, Dataset, + Sample, Ownable ) @@ -61,6 +62,19 @@ Note that we store the provided dataset_id in a variable for later use. Also note the `sourceFolder`. This is a folder on the file system that SciCat has access to, and will contain the files for this `Dataset`. +Proposals and instruments have to be created by an administrator. A sample with `sampleId="gargleblaster"` can be created like this: +```python +sample = Sample( + sampleId="gargleblaster", + owner="Chamber of Commerce", + description="A legendary drink.", + sampleCharacteristics={"Flavour": "Unknown, but potent"}, + isPublished=False, + **ownable.dict() +) +sample_id = client.upload_sample(sample) # sample_id == "gargleblaster" +``` + ## Upload a Datablock ```python diff --git a/pyscicat/client.py b/pyscicat/client.py index 4d9fbc7..14bcdf8 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -419,6 +419,7 @@ def datasets_attachment_create( def samples_create(self, sample: Sample) -> str: """ Create a new sample or update an existing one. + An error is raised when a sample with the same sampleId already exists. This function is also accessible as upload_sample. @@ -450,6 +451,7 @@ def instruments_create(self, instrument: Instrument): """ Create a new instrument or update an existing one. Note that in SciCat admin rights are required to upload instruments. + An error is raised when an instrument with the same pid already exists. This function is also accessible as upload_instrument. @@ -481,6 +483,7 @@ def proposals_create(self, proposal: Proposal): """ Create a new proposal or update an existing one. Note that in SciCat admin rights are required to upload proposals. + An error is raised when a proposal with the same proposalId already exists. This function is also accessible as upload_proposal. From bfed3c88c61e310655fbae041eef36e34dfb1991 Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 17:30:00 +0100 Subject: [PATCH 5/9] Implement update functions for proposals, samples and instruments --- pyscicat/client.py | 100 +++++++++++++++++++++++++++++++++- pyscicat/tests/test_client.py | 17 +++++- 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/pyscicat/client.py b/pyscicat/client.py index 14bcdf8..7bb7c70 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -431,7 +431,7 @@ def samples_create(self, sample: Sample) -> str: Returns ------- str - id of the newly created sample + ID of the newly created sample Raises ------ @@ -447,6 +447,38 @@ def samples_create(self, sample: Sample) -> str: upload_sample = samples_create + def update_sample(self, sample: Sample, sampleId: str = None) -> str: + """Updates an existing sample + + Parameters + ---------- + sample : Sample + Sample to update + + sampleId + ID of sample being updated. By default, ID is taken from sample parameter. + + Returns + ------- + str + ID of the sample + + Raises + ------ + ScicatCommError + Raises if a non-20x message is returned + """ + if sampleId is None: + assert sample.sampleId is not None, "sampleId should not be None" + sampleId = sample.sampleId + sample.sampleId = None + return self._call_endpoint( + cmd="patch", + endpoint=f"Samples/{quote_plus(sampleId)}", + data=sample, + operation="update_sample", + ).get("sampleId") + def instruments_create(self, instrument: Instrument): """ Create a new instrument or update an existing one. @@ -479,6 +511,39 @@ def instruments_create(self, instrument: Instrument): upload_instrument = instruments_create + def update_instrument(self, instrument: Instrument, pid: str = None) -> str: + """Updates an existing instrument. + Note that in SciCat admin rights are required to upload instruments. + + Parameters + ---------- + instrument : Instrument + Instrument to update + + pid + pid (or unique identifier) of instrument being updated. By default, pid is taken from instrument parameter. + + Returns + ------- + str + ID of the instrument + + Raises + ------ + ScicatCommError + Raises if a non-20x message is returned + """ + if pid is None: + assert instrument.pid is not None, "pid should not be None" + pid = instrument.pid + instrument.pid = None + return self._call_endpoint( + cmd="patch", + endpoint=f"Instruments/{quote_plus(pid)}", + data=instrument, + operation="update_instrument", + ).get("pid") + def proposals_create(self, proposal: Proposal): """ Create a new proposal or update an existing one. @@ -511,6 +576,39 @@ def proposals_create(self, proposal: Proposal): upload_proposal = proposals_create + def update_proposal(self, proposal: Proposal, proposalId: str = None) -> str: + """Updates an existing proposal. + Note that in SciCat admin rights are required to upload proposals. + + Parameters + ---------- + proposal : Proposal + Proposal to update + + proposalId + ID of proposal being updated. By default, this is taken from proposal parameter. + + Returns + ------- + str + ID of the proposal + + Raises + ------ + ScicatCommError + Raises if a non-20x message is returned + """ + if proposalId is None: + assert proposal.proposalId is not None, "proposalId should not be None" + proposalId = proposal.proposalId + proposal.proposalId = None + return self._call_endpoint( + cmd="patch", + endpoint=f"Proposals/{quote_plus(proposalId)}", + data=proposal, + operation="update_proposal", + ).get("proposalId") + def datasets_find( self, skip: int = 0, limit: int = 25, query_fields: Optional[dict] = None ) -> Optional[dict]: diff --git a/pyscicat/tests/test_client.py b/pyscicat/tests/test_client.py index 685033b..3bc535f 100644 --- a/pyscicat/tests/test_client.py +++ b/pyscicat/tests/test_client.py @@ -35,6 +35,13 @@ def add_mock_requests(mock_request): mock_request.post(local_url + "Instruments", json={"pid": "earth"}) mock_request.post(local_url + "Proposals", json={"proposalId": "deepthought"}) mock_request.post(local_url + "Samples", json={"sampleId": "gargleblaster"}) + mock_request.patch(local_url + "Instruments/earth", json={"pid": "earth"}) + mock_request.patch( + local_url + "Proposals/deepthought", json={"proposalId": "deepthought"} + ) + mock_request.patch( + local_url + "Samples/gargleblaster", json={"sampleId": "gargleblaster"} + ) mock_request.post(local_url + "RawDatasets/replaceOrCreate", json={"pid": "42"}) mock_request.patch( @@ -79,13 +86,18 @@ def test_scicat_ingest(): ) assert scicat.upload_instrument(instrument) == "earth" assert scicat.instruments_create(instrument) == "earth" + assert scicat.update_instrument(instrument) == "earth" # Proposal proposal = Proposal( - proposalId="deepthought", title="Deepthought", **ownable.dict() + proposalId="deepthought", + title="Deepthought", + email="deepthought@viltvodle.com", + **ownable.dict() ) assert scicat.upload_proposal(proposal) == "deepthought" assert scicat.proposals_create(proposal) == "deepthought" + assert scicat.update_proposal(proposal) == "deepthought" # Sample sample = Sample( @@ -95,7 +107,8 @@ def test_scicat_ingest(): **ownable.dict() ) assert scicat.upload_sample(sample) == "gargleblaster" - assert scicat.samples_create(proposal) == "gargleblaster" + assert scicat.samples_create(sample) == "gargleblaster" + assert scicat.update_sample(sample) == "gargleblaster" # RawDataset dataset = RawDataset( From a3a4276731e9d9e6259d2b1dd6368bf8525fc7a3 Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 18:16:59 +0100 Subject: [PATCH 6/9] Fix documentation --- pyscicat/client.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pyscicat/client.py b/pyscicat/client.py index 7bb7c70..b28a743 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -418,7 +418,7 @@ def datasets_attachment_create( def samples_create(self, sample: Sample) -> str: """ - Create a new sample or update an existing one. + Create a new sample. An error is raised when a sample with the same sampleId already exists. This function is also accessible as upload_sample. @@ -467,6 +467,9 @@ def update_sample(self, sample: Sample, sampleId: str = None) -> str: ------ ScicatCommError Raises if a non-20x message is returned + + AssertionError + Raises if no ID is provided """ if sampleId is None: assert sample.sampleId is not None, "sampleId should not be None" @@ -481,7 +484,7 @@ def update_sample(self, sample: Sample, sampleId: str = None) -> str: def instruments_create(self, instrument: Instrument): """ - Create a new instrument or update an existing one. + Create a new instrument. Note that in SciCat admin rights are required to upload instruments. An error is raised when an instrument with the same pid already exists. This function is also accessible as upload_instrument. @@ -532,6 +535,9 @@ def update_instrument(self, instrument: Instrument, pid: str = None) -> str: ------ ScicatCommError Raises if a non-20x message is returned + + AssertionError + Raises if no ID is provided """ if pid is None: assert instrument.pid is not None, "pid should not be None" @@ -546,7 +552,7 @@ def update_instrument(self, instrument: Instrument, pid: str = None) -> str: def proposals_create(self, proposal: Proposal): """ - Create a new proposal or update an existing one. + Create a new proposal. Note that in SciCat admin rights are required to upload proposals. An error is raised when a proposal with the same proposalId already exists. This function is also accessible as upload_proposal. @@ -560,7 +566,7 @@ def proposals_create(self, proposal: Proposal): Returns ------- str - id of the newly created proposal + ID of the newly created proposal Raises ------ @@ -597,6 +603,9 @@ def update_proposal(self, proposal: Proposal, proposalId: str = None) -> str: ------ ScicatCommError Raises if a non-20x message is returned + + AssertionError + Raises if no ID is provided """ if proposalId is None: assert proposal.proposalId is not None, "proposalId should not be None" From dc86102487e55ee89266dcdb058daf7861dff576 Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Fri, 27 Jan 2023 19:31:56 +0100 Subject: [PATCH 7/9] Flake8 compliance --- pyscicat/client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyscicat/client.py b/pyscicat/client.py index b28a743..37adaaf 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -524,7 +524,8 @@ def update_instrument(self, instrument: Instrument, pid: str = None) -> str: Instrument to update pid - pid (or unique identifier) of instrument being updated. By default, pid is taken from instrument parameter. + pid (or unique identifier) of instrument being updated. + By default, pid is taken from instrument parameter. Returns ------- From 1ee0bd3e64d41ef436b3ec62be9c0a11804b8c70 Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Mon, 30 Jan 2023 11:07:12 +0100 Subject: [PATCH 8/9] Update-function of the form _update --- pyscicat/client.py | 12 ++++++------ pyscicat/tests/test_client.py | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pyscicat/client.py b/pyscicat/client.py index 37adaaf..d381e1f 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -447,7 +447,7 @@ def samples_create(self, sample: Sample) -> str: upload_sample = samples_create - def update_sample(self, sample: Sample, sampleId: str = None) -> str: + def samples_update(self, sample: Sample, sampleId: str = None) -> str: """Updates an existing sample Parameters @@ -479,7 +479,7 @@ def update_sample(self, sample: Sample, sampleId: str = None) -> str: cmd="patch", endpoint=f"Samples/{quote_plus(sampleId)}", data=sample, - operation="update_sample", + operation="samples_update", ).get("sampleId") def instruments_create(self, instrument: Instrument): @@ -514,7 +514,7 @@ def instruments_create(self, instrument: Instrument): upload_instrument = instruments_create - def update_instrument(self, instrument: Instrument, pid: str = None) -> str: + def instruments_update(self, instrument: Instrument, pid: str = None) -> str: """Updates an existing instrument. Note that in SciCat admin rights are required to upload instruments. @@ -548,7 +548,7 @@ def update_instrument(self, instrument: Instrument, pid: str = None) -> str: cmd="patch", endpoint=f"Instruments/{quote_plus(pid)}", data=instrument, - operation="update_instrument", + operation="instruments_update", ).get("pid") def proposals_create(self, proposal: Proposal): @@ -583,7 +583,7 @@ def proposals_create(self, proposal: Proposal): upload_proposal = proposals_create - def update_proposal(self, proposal: Proposal, proposalId: str = None) -> str: + def proposals_update(self, proposal: Proposal, proposalId: str = None) -> str: """Updates an existing proposal. Note that in SciCat admin rights are required to upload proposals. @@ -616,7 +616,7 @@ def update_proposal(self, proposal: Proposal, proposalId: str = None) -> str: cmd="patch", endpoint=f"Proposals/{quote_plus(proposalId)}", data=proposal, - operation="update_proposal", + operation="proposals_update", ).get("proposalId") def datasets_find( diff --git a/pyscicat/tests/test_client.py b/pyscicat/tests/test_client.py index 3bc535f..6d036f0 100644 --- a/pyscicat/tests/test_client.py +++ b/pyscicat/tests/test_client.py @@ -86,7 +86,7 @@ def test_scicat_ingest(): ) assert scicat.upload_instrument(instrument) == "earth" assert scicat.instruments_create(instrument) == "earth" - assert scicat.update_instrument(instrument) == "earth" + assert scicat.instruments_update(instrument) == "earth" # Proposal proposal = Proposal( @@ -97,7 +97,7 @@ def test_scicat_ingest(): ) assert scicat.upload_proposal(proposal) == "deepthought" assert scicat.proposals_create(proposal) == "deepthought" - assert scicat.update_proposal(proposal) == "deepthought" + assert scicat.proposals_update(proposal) == "deepthought" # Sample sample = Sample( @@ -108,7 +108,7 @@ def test_scicat_ingest(): ) assert scicat.upload_sample(sample) == "gargleblaster" assert scicat.samples_create(sample) == "gargleblaster" - assert scicat.update_sample(sample) == "gargleblaster" + assert scicat.samples_update(sample) == "gargleblaster" # RawDataset dataset = RawDataset( From 336fc874ec7c7c618133959ddc0d7ad58047970c Mon Sep 17 00:00:00 2001 From: Martin Voigt Date: Mon, 30 Jan 2023 11:34:09 +0100 Subject: [PATCH 9/9] Rename update_dataset() to datasets_update() in client. --- pyscicat/client.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pyscicat/client.py b/pyscicat/client.py index d381e1f..9d3e05a 100644 --- a/pyscicat/client.py +++ b/pyscicat/client.py @@ -211,7 +211,7 @@ def datasets_raw_replace(self, dataset: Dataset) -> str: This function was renamed. It is still accessible with the original name for backward compatibility The original names were repalce_raw_dataset and upload_raw_dataset - THis function is obsolete and it will be removed in future releases + This function is obsolete and it will be removed in future releases Parameters ---------- @@ -272,8 +272,11 @@ def datasets_derived_replace(self, dataset: Dataset) -> str: operation="datasets_derived_replace", ).get("pid") - def update_dataset(self, dataset: Dataset, pid: str) -> str: + def datasets_update(self, dataset: Dataset, pid: str) -> str: """Updates an existing dataset + This function was renamed. + It is still accessible with the original name for backward compatibility + The original name was update_dataset. Parameters ---------- @@ -296,9 +299,15 @@ def update_dataset(self, dataset: Dataset, pid: str) -> str: cmd="patch", endpoint=f"Datasets/{quote_plus(pid)}", data=dataset, - operation="update_dataset", + operation="datasets_update", ).get("pid") + """ + Update a dataset + Original name, kept for for backward compatibility + """ + update_dataset = datasets_update + def datasets_datablock_create( self, datablock: Datablock, datasetType: str = "RawDatasets" ) -> dict: