From 38973aafcd1d6f349eb1a3e02242d58e34e207ba Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Tue, 28 Nov 2023 09:24:29 +0100
Subject: [PATCH 1/8] Fix typing errors

- Fix mypy errors in all files except scientific_metadata.py.
  Some signatures changed, but hopefully in a way that preserves
  the original intent  (eg removing defaults).
- Ran black formatting
---
 examples/ingest_dataset.py                   |   4 +-
 examples/ingestion_simulation_dataset_ess.py |   2 +-
 pyscicat/client.py                           | 118 +++++++++++--------
 pyscicat/hdf5/h5tools.py                     |   4 +-
 pyscicat/model.py                            |   2 +-
 tests/test_pyscicat/test_client.py           |  21 ++--
 tests/tests_integration/tests_integration.py |  25 ++--
 7 files changed, 100 insertions(+), 76 deletions(-)

diff --git a/examples/ingest_dataset.py b/examples/ingest_dataset.py
index 55393fb..de166f7 100644
--- a/examples/ingest_dataset.py
+++ b/examples/ingest_dataset.py
@@ -31,7 +31,7 @@
     sourceFolder="/foo/bar",
     scientificMetadata={"a": "field"},
     sampleId="gargleblaster",
-    **ownable.dict()
+    **ownable.dict(),
 )
 dataset_id = scicat.upload_raw_dataset(dataset)
 
@@ -47,6 +47,6 @@
     datasetId=dataset_id,
     thumbnail=encode_thumbnail(thumb_path),
     caption="scattering image",
-    **ownable.dict()
+    **ownable.dict(),
 )
 scicat.upload_attachment(attachment)
diff --git a/examples/ingestion_simulation_dataset_ess.py b/examples/ingestion_simulation_dataset_ess.py
index 96c57c6..cbb7009 100644
--- a/examples/ingestion_simulation_dataset_ess.py
+++ b/examples/ingestion_simulation_dataset_ess.py
@@ -68,7 +68,7 @@
         pyScModel.DataFile(**file)
         for file in dataset_information["orig_datablock"]["dataFileList"]
     ],
-    **ownable.dict()
+    **ownable.dict(),
 )
 
 # create origDatablock associated with dataset in SciCat
diff --git a/pyscicat/client.py b/pyscicat/client.py
index 120471e..3d422aa 100644
--- a/pyscicat/client.py
+++ b/pyscicat/client.py
@@ -50,12 +50,12 @@ class ScicatClient:
     def __init__(
         self,
         base_url: str,
-        token: str = False,
-        username: str = None,
-        password: str = None,
-        timeout_seconds: int = None,
+        token: Optional[str] = None,
+        username: Optional[str] = None,
+        password: Optional[str] = None,
+        timeout_seconds: Optional[int] = None,
     ):
-        """Initialize a new instance. This method attempts to create a tokenad_a
+        """Initialize a new instance. This method attempts to create a token
         from the provided username and password
 
         Parameters
@@ -87,7 +87,9 @@ def __init__(
             self._token = get_token(self._base_url, self._username, self._password)
         self._headers["Authorization"] = "Bearer {}".format(self._token)
 
-    def _send_to_scicat(self, cmd: str, endpoint: str, data: BaseModel = None):
+    def _send_to_scicat(
+        self, cmd: str, endpoint: str, data: Optional[BaseModel] = None
+    ):
         """sends a command to the SciCat API server using url and token, returns the response JSON
         Get token with the getToken method"""
         return requests.request(
@@ -105,7 +107,7 @@ def _call_endpoint(
         self,
         cmd: str,
         endpoint: str,
-        data: BaseModel = None,
+        data: Optional[BaseModel] = None,
         operation: str = "",
     ) -> Optional[dict]:
         response = self._send_to_scicat(cmd=cmd, endpoint=endpoint, data=data)
@@ -144,9 +146,11 @@ def datasets_create(self, dataset: Dataset) -> str:
         ScicatCommError
             Raises if a non-20x message is returned
         """
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="post", endpoint="Datasets", data=dataset, operation="datasets_create"
-        ).get("pid")
+        )
+        assert result and "pid" in result and isinstance(result["pid"], str)
+        return result["pid"]
 
     """
         Upload a new dataset
@@ -178,12 +182,14 @@ def datasets_update(self, dataset: Dataset, pid: str) -> str:
         ScicatCommError
             Raises if a non-20x message is returned
         """
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="patch",
             endpoint=f"Datasets/{quote_plus(pid)}",
             data=dataset,
             operation="datasets_update",
-        ).get("pid")
+        )
+        assert result and "pid" in result and isinstance(result["pid"], str)
+        return result["pid"]
 
     """
         Update a dataset
@@ -217,12 +223,14 @@ def datasets_origdatablock_create(
 
         """
         endpoint = f"Datasets/{quote_plus(dataset_id)}/origdatablocks"
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="post",
             endpoint=endpoint,
             data=datablockDto,
             operation="datasets_origdatablock_create",
         )
+        assert isinstance(result, dict)
+        return result
 
     """
         Create a new SciCat Dataset OrigDatablock
@@ -255,12 +263,14 @@ def datasets_attachment_create(
             Raises if a non-20x message is returned
         """
         endpoint = f"{datasetType}/{quote_plus(attachment.datasetId)}/attachments"
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="post",
             endpoint=endpoint,
             data=attachment,
             operation="datasets_attachment_create",
         )
+        assert isinstance(result, dict)
+        return result
 
     """
         Create a new attachement for a dataset
@@ -291,16 +301,18 @@ def samples_create(self, sample: Sample) -> str:
         ScicatCommError
             Raises if a non-20x message is returned
         """
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="post",
             endpoint="Samples",
             data=sample,
             operation="samples_create",
-        ).get("sampleId")
+        )
+        assert result and "sampleId" in result and isinstance(result["sampleId"], str)
+        return result["sampleId"]
 
     upload_sample = samples_create
 
-    def samples_update(self, sample: Sample, sampleId: str = None) -> str:
+    def samples_update(self, sample: Sample, sampleId: Optional[str] = None) -> str:
         """Updates an existing sample
 
         Parameters
@@ -328,12 +340,14 @@ def samples_update(self, sample: Sample, sampleId: str = None) -> str:
             assert sample.sampleId is not None, "sampleId should not be None"
             sampleId = sample.sampleId
         sample.sampleId = None
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="patch",
             endpoint=f"Samples/{quote_plus(sampleId)}",
             data=sample,
             operation="samples_update",
-        ).get("sampleId")
+        )
+        assert result and "sampleId" in result and isinstance(result["sampleId"], str)
+        return result["sampleId"]
 
     def instruments_create(self, instrument: Instrument):
         """
@@ -358,16 +372,20 @@ def instruments_create(self, instrument: Instrument):
         ScicatCommError
             Raises if a non-20x message is returned
         """
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="post",
             endpoint="Instruments",
             data=instrument,
             operation="instruments_create",
-        ).get("pid")
+        )
+        assert result and "pid" in result and isinstance(result["pid"], str)
+        return result["pid"]
 
     upload_instrument = instruments_create
 
-    def instruments_update(self, instrument: Instrument, pid: str = None) -> str:
+    def instruments_update(
+        self, instrument: Instrument, pid: Optional[str] = None
+    ) -> str:
         """Updates an existing instrument.
         Note that in SciCat admin rights are required to upload instruments.
 
@@ -397,12 +415,14 @@ def instruments_update(self, instrument: Instrument, pid: str = None) -> str:
             assert instrument.pid is not None, "pid should not be None"
             pid = instrument.pid
         instrument.pid = None
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="patch",
             endpoint=f"Instruments/{quote_plus(pid)}",
             data=instrument,
             operation="instruments_update",
-        ).get("pid")
+        )
+        assert result and "pid" in result and isinstance(result["pid"], str)
+        return result["pid"]
 
     def proposals_create(self, proposal: Proposal):
         """
@@ -427,16 +447,22 @@ def proposals_create(self, proposal: Proposal):
         ScicatCommError
             Raises if a non-20x message is returned
         """
-        return self._call_endpoint(
+        result = self._call_endpoint(
             cmd="post",
             endpoint="Proposals",
             data=proposal,
             operation="proposals_create",
-        ).get("proposalId")
+        )
+        assert (
+            result and "proposalId" in result and isinstance(result["proposalId"], str)
+        )
+        return result["proposalId"]
 
     upload_proposal = proposals_create
 
-    def proposals_update(self, proposal: Proposal, proposalId: str = None) -> str:
+    def proposals_update(
+        self, proposal: Proposal, proposalId: Optional[str] = None
+    ) -> str:
         """Updates an existing proposal.
         Note that in SciCat admin rights are required to upload proposals.
 
@@ -464,13 +490,17 @@ def proposals_update(self, proposal: Proposal, proposalId: str = None) -> str:
         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(
+        proposal.proposalId = ""
+        result = self._call_endpoint(
             cmd="patch",
             endpoint=f"Proposals/{quote_plus(proposalId)}",
             data=proposal,
             operation="proposals_update",
-        ).get("proposalId")
+        )
+        assert (
+            result and "proposalId" in result and isinstance(result["proposalId"], str)
+        )
+        return result["proposalId"]
 
     def datasets_find(
         self, skip: int = 0, limit: int = 25, query_fields: Optional[dict] = None
@@ -504,8 +534,8 @@ def datasets_find(
         """
         if not query_fields:
             query_fields = {}
-        query_fields = json.dumps(query_fields)
-        query = f'fields={query_fields}&limits={{"skip":{skip},"limit":{limit},"order":"creationTime:desc"}}'
+        query_field_str = json.dumps(query_fields)
+        query = f'fields={query_field_str}&limits={{"skip":{skip},"limit":{limit},"order":"creationTime:desc"}}'
 
         return self._call_endpoint(
             cmd="get",
@@ -546,10 +576,10 @@ def datasets_get_many(self, filter_fields: Optional[dict] = None) -> Optional[di
         filter_fields : dict
             Dictionary of filtering fields. Must be json serializable.
         """
-        if not filter_fields:
+        if filter_fields is None:
             filter_fields = {}
-        filter_fields = json.dumps(filter_fields)
-        endpoint = f'Datasets?filter={{"where":{filter_fields}}}'
+        filter_field_str = json.dumps(filter_fields)
+        endpoint = f'Datasets?filter={{"where":{filter_field_str}}}'
         return self._call_endpoint(
             cmd="get", endpoint=endpoint, operation="datasets_get_many"
         )
@@ -615,7 +645,9 @@ def datasets_get_one(self, pid: str) -> Optional[dict]:
 
     get_dataset_by_pid = datasets_get_one
 
-    def instruments_get_one(self, pid: str = None, name: str = None) -> Optional[dict]:
+    def instruments_get_one(
+        self, pid: Optional[str] = None, name: Optional[str] = None
+    ) -> Optional[dict]:
         """
         Get an instrument by pid or by name.
         If pid is provided it takes priority over name.
@@ -678,7 +710,7 @@ def samples_get_one(self, pid: str) -> Optional[dict]:
 
     get_sample = samples_get_one
 
-    def proposals_get_one(self, pid: str = None) -> Optional[dict]:
+    def proposals_get_one(self, pid: str) -> Optional[dict]:
         """
         Get proposal by pid.
         This function has been renamed. Previous name has been maintained for backward compatibility.
@@ -694,9 +726,7 @@ def proposals_get_one(self, pid: str = None) -> Optional[dict]:
         dict
             The proposal with the requested pid
         """
-        return self._call_endpoint(
-            cmd="get", endpoint=f"Proposals/{quote_plus(pid)}"
-        )
+        return self._call_endpoint(cmd="get", endpoint=f"Proposals/{quote_plus(pid)}")
 
     get_proposal = proposals_get_one
 
@@ -761,7 +791,7 @@ def get_checksum(pathobj):
 
 
 def encode_thumbnail(filename, imType="jpg"):
-    logging.info(f"Creating thumbnail for dataset: {filename}")
+    logger.info(f"Creating thumbnail for dataset: {filename}")
     header = "data:image/{imType};base64,".format(imType=imType)
     with open(filename, "rb") as f:
         data = f.read()
@@ -809,9 +839,7 @@ def _log_in_via_auth_msad(base_url, username, password):
         verify=True,
     )
     if not response.ok:
-        logger.error(
-            f'Error retrieving token for user: {response.json()}'
-        )
+        logger.error(f"Error retrieving token for user: {response.json()}")
         raise ScicatLoginError(response.content)
 
 
@@ -831,7 +859,5 @@ def get_token(base_url, username, password):
     if response.ok:
         return response.json()["access_token"]
 
-    logger.error(
-        f' Failed log in:  {response.json()}'
-    )
+    logger.error(f" Failed log in:  {response.json()}")
     raise ScicatLoginError(response.content)
diff --git a/pyscicat/hdf5/h5tools.py b/pyscicat/hdf5/h5tools.py
index 7239119..93a0efe 100644
--- a/pyscicat/hdf5/h5tools.py
+++ b/pyscicat/hdf5/h5tools.py
@@ -3,13 +3,13 @@
 # author: Sofya Laskina, Brian R. Pauw, I. Bressler
 # date: 2022.01.10
 
-import h5py
+import h5py  # type: ignore
 import logging
 import numpy as np
 from pathlib import Path
 
 
-def h5Get(filename, h5path: str = None, default="none", leaveAsArray=False):
+def h5Get(filename, h5path: str, default="none", leaveAsArray=False):
     """
     Gets a single value or attribute from an HDF5 file, with added error checking and default handling.
     h5path is the string representation of the location in the hdf5 file, e.g. '/sasentry1/sasdata1/I'.
diff --git a/pyscicat/model.py b/pyscicat/model.py
index 4cc43e2..53046ed 100644
--- a/pyscicat/model.py
+++ b/pyscicat/model.py
@@ -194,7 +194,7 @@ class Datablock(Ownable):
 
     packedSize: Optional[int] = None
     chkAlg: Optional[int] = None
-    version: str = None
+    version: str = ""
     instrumentGroup: Optional[str] = None
     datasetId: str
 
diff --git a/tests/test_pyscicat/test_client.py b/tests/test_pyscicat/test_client.py
index 364feea..70c3ada 100644
--- a/tests/test_pyscicat/test_client.py
+++ b/tests/test_pyscicat/test_client.py
@@ -94,7 +94,7 @@ def test_scicat_ingest():
             proposalId="deepthought",
             title="Deepthought",
             email="deepthought@viltvodle.com",
-            **ownable.dict()
+            **ownable.dict(),
         )
         assert scicat.upload_proposal(proposal) == "deepthought"
         assert scicat.proposals_create(proposal) == "deepthought"
@@ -105,7 +105,7 @@ def test_scicat_ingest():
             sampleId="gargleblaster",
             description="Gargleblaster",
             sampleCharacteristics={"a": "field"},
-            **ownable.dict()
+            **ownable.dict(),
         )
         assert scicat.upload_sample(sample) == "gargleblaster"
         assert scicat.samples_create(sample) == "gargleblaster"
@@ -127,7 +127,7 @@ def test_scicat_ingest():
             sourceFolder="/foo/bar",
             scientificMetadata={"a": "field"},
             sampleId="gargleblaster",
-            **ownable.dict()
+            **ownable.dict(),
         )
         dataset_id = scicat.upload_new_dataset(dataset)
         assert dataset_id == "42"
@@ -143,7 +143,6 @@ def test_scicat_ingest():
             size=42,
             datasetId=dataset_id,
             dataFileList=[data_file],
-
         )
         scicat.upload_dataset_origdatablock(dataset_id, data_block_dto)
 
@@ -152,7 +151,7 @@ def test_scicat_ingest():
             datasetId=dataset_id,
             thumbnail=encode_thumbnail(thumb_path),
             caption="scattering image",
-            **ownable.dict()
+            **ownable.dict(),
         )
         scicat.upload_attachment(attachment)
 
@@ -186,11 +185,7 @@ def test_get_dataset():
 
 def test_get_nonexistent_dataset():
     with requests_mock.Mocker() as mock_request:
-        mock_request.get(
-            local_url + "datasets/74",
-            status_code=200,
-            content=b''
-        )
+        mock_request.get(local_url + "datasets/74", status_code=200, content=b"")
         client = from_token(base_url=local_url, token="a_token")
         assert client.datasets_get_one("74") is None
 
@@ -220,12 +215,12 @@ def test_initializers():
 
         client = from_token(local_url, "a_token")
         assert client._token == "a_token"
-        assert client._headers['Authorization'] == "Bearer a_token"
+        assert client._headers["Authorization"] == "Bearer a_token"
 
         client = from_credentials(local_url, "Zaphod", "heartofgold")
         assert client._token == "a_token"
-        assert client._headers['Authorization'] == "Bearer a_token"
+        assert client._headers["Authorization"] == "Bearer a_token"
 
         client = ScicatClient(local_url, "a_token")
         assert client._token == "a_token"
-        assert client._headers['Authorization'] == "Bearer a_token"
+        assert client._headers["Authorization"] == "Bearer a_token"
diff --git a/tests/tests_integration/tests_integration.py b/tests/tests_integration/tests_integration.py
index 86ad83c..34e3d6d 100644
--- a/tests/tests_integration/tests_integration.py
+++ b/tests/tests_integration/tests_integration.py
@@ -18,10 +18,12 @@
 SCICAT_PASSWORD - the password for your scicat user.
 """
 
-sci_clie = ScicatClient(base_url=os.environ["BASE_URL"],
-                        token=None,
-                        username=os.environ["SCICAT_USER"],
-                        password=os.environ["SCICAT_PASSWORD"])
+sci_clie = ScicatClient(
+    base_url=os.environ["BASE_URL"],
+    token=None,
+    username=os.environ["SCICAT_USER"],
+    password=os.environ["SCICAT_PASSWORD"],
+)
 
 
 def test_client():
@@ -53,14 +55,13 @@ def test_upload_dataset():
         techniques=[],
         numberOfFiles=0,
         numberOfFilesArchived=0,
-        **ownable.dict()
+        **ownable.dict(),
     )
 
     sci_clie.upload_new_dataset(payload)
 
 
 def test_get_dataset():
-
     datasets = sci_clie.get_datasets({"ownerGroup": "ingestor"})
 
     for dataset in datasets:
@@ -68,10 +69,12 @@ def test_get_dataset():
 
 
 def test_update_dataset():
-    sci_clie = ScicatClient(base_url=os.environ["BASE_URL"],
-                            token=None,
-                            username=os.environ["SCICAT_USER"],
-                            password=os.environ["SCICAT_PASSWORD"])
+    sci_clie = ScicatClient(
+        base_url=os.environ["BASE_URL"],
+        token=None,
+        username=os.environ["SCICAT_USER"],
+        password=os.environ["SCICAT_PASSWORD"],
+    )
 
     datasets = sci_clie.get_datasets({})
     pid = datasets[0]["pid"]
@@ -89,6 +92,6 @@ def test_update_dataset():
         sourceFolder="/foo/bar",
         scientificMetadata={"a": "field"},
         sampleId="gargleblaster",
-        accessGroups=["Vogons"]
+        accessGroups=["Vogons"],
     )
     sci_clie.update_dataset(payload, pid)

From 3406f6e4b3f040fd5f5d0e73b5dd26a3d3753df7 Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Tue, 28 Nov 2023 09:34:27 +0100
Subject: [PATCH 2/8] Run isort

---
 examples/ingest_dataset.py                   |  2 +-
 examples/ingestion_simulation_dataset_ess.py |  2 +-
 pyproject.toml                               |  4 ++++
 pyscicat/client.py                           | 12 +++++-------
 pyscicat/hdf5/h5tools.py                     |  5 +++--
 pyscicat/hdf5/scientific_metadata.py         |  8 +++++---
 pyscicat/model.py                            |  2 +-
 tests/test_hdf5/test_hdf5sct.py              |  3 ++-
 tests/test_pyscicat/test_client.py           | 10 +++++-----
 tests/tests_integration/tests_integration.py |  6 +++---
 10 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/examples/ingest_dataset.py b/examples/ingest_dataset.py
index de166f7..6fc59c9 100644
--- a/examples/ingest_dataset.py
+++ b/examples/ingest_dataset.py
@@ -1,7 +1,7 @@
 from datetime import datetime
 from pathlib import Path
 
-from pyscicat.client import encode_thumbnail, ScicatClient
+from pyscicat.client import ScicatClient, encode_thumbnail
 from pyscicat.model import Attachment, Datablock, DataFile, Dataset, Ownable
 
 # Create a client object. The account used should have the ingestor role in SciCat
diff --git a/examples/ingestion_simulation_dataset_ess.py b/examples/ingestion_simulation_dataset_ess.py
index cbb7009..444e637 100644
--- a/examples/ingestion_simulation_dataset_ess.py
+++ b/examples/ingestion_simulation_dataset_ess.py
@@ -18,10 +18,10 @@
 
 # libraries
 import json
+
 import pyscicat.client as pyScClient
 import pyscicat.model as pyScModel
 
-
 # scicat configuration file
 # includes scicat instance URL
 # scicat user and password
diff --git a/pyproject.toml b/pyproject.toml
index f6505bf..b4a6c40 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -41,6 +41,8 @@ dev = [
     "twine",
     "black",
     "requests_mock",
+    "isort",
+    "mypy",
 ]
 docs = [
     "ipython",
@@ -74,6 +76,8 @@ exclude = '''
 )/
 '''
 
+[tool.isort]
+profile = "black"
 
 [tool.hatch]
 version.source = "vcs"
diff --git a/pyscicat/client.py b/pyscicat/client.py
index 3d422aa..95d1fc4 100644
--- a/pyscicat/client.py
+++ b/pyscicat/client.py
@@ -1,16 +1,14 @@
-from datetime import datetime
-import enum
-
 import base64
+import enum
 import hashlib
-import logging
 import json
+import logging
+from datetime import datetime
 from typing import Optional
-from urllib.parse import urljoin, quote_plus
+from urllib.parse import quote_plus, urljoin
 
-from pydantic import BaseModel
 import requests
-
+from pydantic import BaseModel
 
 from pyscicat.model import (
     Attachment,
diff --git a/pyscicat/hdf5/h5tools.py b/pyscicat/hdf5/h5tools.py
index 93a0efe..c588685 100644
--- a/pyscicat/hdf5/h5tools.py
+++ b/pyscicat/hdf5/h5tools.py
@@ -3,11 +3,12 @@
 # author: Sofya Laskina, Brian R. Pauw, I. Bressler
 # date: 2022.01.10
 
-import h5py  # type: ignore
 import logging
-import numpy as np
 from pathlib import Path
 
+import h5py  # type: ignore
+import numpy as np
+
 
 def h5Get(filename, h5path: str, default="none", leaveAsArray=False):
     """
diff --git a/pyscicat/hdf5/scientific_metadata.py b/pyscicat/hdf5/scientific_metadata.py
index 8fe8533..c35144e 100644
--- a/pyscicat/hdf5/scientific_metadata.py
+++ b/pyscicat/hdf5/scientific_metadata.py
@@ -3,15 +3,17 @@
 # author: Sofya Laskina, Brian R. Pauw
 # date: 2022.01.10
 
+import logging
+from collections import abc
+from pathlib import Path
+
 # from unittest import skip # not sure where this import comes from
 import h5py
 
 # flake8: noqa: F401
 import hdf5plugin  # ESRF's library that extends the read functionality of HDF5 files
+
 from pyscicat.hdf5.h5tools import h5py_casting
-import logging
-from pathlib import Path
-from collections import abc
 
 
 def update_deep(dictionary: dict, path_update: dict) -> dict:
diff --git a/pyscicat/model.py b/pyscicat/model.py
index 53046ed..84c5a15 100644
--- a/pyscicat/model.py
+++ b/pyscicat/model.py
@@ -1,7 +1,7 @@
 import enum
 
 # from re import L
-from typing import List, Dict, Optional
+from typing import Dict, List, Optional
 
 from pydantic import BaseModel
 
diff --git a/tests/test_hdf5/test_hdf5sct.py b/tests/test_hdf5/test_hdf5sct.py
index 3f3af64..7db2c16 100644
--- a/tests/test_hdf5/test_hdf5sct.py
+++ b/tests/test_hdf5/test_hdf5sct.py
@@ -2,9 +2,10 @@
 # might be related to the change of import style for Python 3.7+. Tested on Python 3.9 at 20220114
 from pathlib import Path
 
+from pyscicat.hdf5.h5tools import h5Get, h5GetDict
+
 # these packages are failing to import in McHat if they are not loaded here:
 from pyscicat.hdf5.scientific_metadata import scientific_metadata
-from pyscicat.hdf5.h5tools import h5Get, h5GetDict
 
 
 def test_readValue():
diff --git a/tests/test_pyscicat/test_client.py b/tests/test_pyscicat/test_client.py
index 70c3ada..ce35edd 100644
--- a/tests/test_pyscicat/test_client.py
+++ b/tests/test_pyscicat/test_client.py
@@ -3,25 +3,25 @@
 
 import pytest
 import requests_mock
+
 from pyscicat.client import (
+    ScicatClient,
+    ScicatCommError,
+    encode_thumbnail,
     from_credentials,
     from_token,
-    encode_thumbnail,
     get_file_mod_time,
     get_file_size,
-    ScicatClient,
-    ScicatCommError,
 )
-
 from pyscicat.model import (
     Attachment,
     CreateDatasetOrigDatablockDto,
     DataFile,
     Instrument,
+    Ownable,
     Proposal,
     RawDataset,
     Sample,
-    Ownable,
 )
 
 local_url = "http://localhost:3000/api/v3/"
diff --git a/tests/tests_integration/tests_integration.py b/tests/tests_integration/tests_integration.py
index 34e3d6d..ca3452e 100644
--- a/tests/tests_integration/tests_integration.py
+++ b/tests/tests_integration/tests_integration.py
@@ -1,8 +1,8 @@
-from pyscicat.client import ScicatClient
-from pyscicat.model import RawDataset, Ownable
-from datetime import datetime
 import os
+from datetime import datetime
 
+from pyscicat.client import ScicatClient
+from pyscicat.model import Ownable, RawDataset
 
 """
 These test_pyscicat do not use mocks and are designed to connect

From 09065793f367e62f17dd486e07bc0e9bac281a9a Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Tue, 28 Nov 2023 09:50:37 +0100
Subject: [PATCH 3/8] Add black, isort, and mypy to github tests

scientific_metadata.py is excluded from mypy temporarily.
---
 .github/workflows/testing.yml | 12 ++++++++++++
 pyproject.toml                |  6 +++++-
 pyscicat/client.py            |  9 +++++----
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml
index b9a8056..4e4db9b 100644
--- a/.github/workflows/testing.yml
+++ b/.github/workflows/testing.yml
@@ -43,6 +43,18 @@ jobs:
       run: |
         set -vxeuo pipefail
         python -m flake8
+    
+    - name: Lint with black
+      shell: bash -l {0}
+      run: black --check .
+      
+    - name: Lint with isort
+      shell: bash -l {0}
+      run: isort --check .
+      
+    - name: Lint with mypy
+      shell: bash -l {0}
+      run: mypy --check pyscicat
       
     - name: Test with pytest
       shell: bash -l {0}
diff --git a/pyproject.toml b/pyproject.toml
index b4a6c40..63dd5b2 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -81,4 +81,8 @@ profile = "black"
 
 [tool.hatch]
 version.source = "vcs"
-build.hooks.vcs.version-file = "pyscicat/_version.py"
\ No newline at end of file
+build.hooks.vcs.version-file = "pyscicat/_version.py"
+
+[tool.mypy]
+# TODO Add type annotations and include this file
+exclude = "pyscicat/hdf5/scientific_metadata.py"
\ No newline at end of file
diff --git a/pyscicat/client.py b/pyscicat/client.py
index 95d1fc4..420cbd0 100644
--- a/pyscicat/client.py
+++ b/pyscicat/client.py
@@ -5,6 +5,7 @@
 import logging
 from datetime import datetime
 from typing import Optional
+from pathlib import Path
 from urllib.parse import quote_plus, urljoin
 
 import requests
@@ -777,13 +778,13 @@ def datasets_delete(self, pid: str) -> Optional[dict]:
     delete_dataset = datasets_delete
 
 
-def get_file_size(pathobj):
+def get_file_size(pathobj: Path):
     filesize = pathobj.lstat().st_size
     return filesize
 
 
-def get_checksum(pathobj):
-    with open(pathobj) as file_to_check:
+def get_checksum(pathobj: Path):
+    with open(pathobj,'rb') as file_to_check:
         # pipe contents of the file through
         return hashlib.md5(file_to_check.read()).hexdigest()
 
@@ -798,7 +799,7 @@ def encode_thumbnail(filename, imType="jpg"):
     return header + dataStr
 
 
-def get_file_mod_time(pathobj):
+def get_file_mod_time(pathobj: Path):
     # may only work on WindowsPath objects...
     # timestamp = pathobj.lstat().st_mtime
     return str(datetime.fromtimestamp(pathobj.lstat().st_mtime))

From 18af4e32d5c6c97281ecfb3c355f98c7d824eb47 Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Tue, 28 Nov 2023 09:59:14 +0100
Subject: [PATCH 4/8] Format

---
 pyscicat/client.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pyscicat/client.py b/pyscicat/client.py
index 420cbd0..4cdbe6a 100644
--- a/pyscicat/client.py
+++ b/pyscicat/client.py
@@ -4,8 +4,8 @@
 import json
 import logging
 from datetime import datetime
-from typing import Optional
 from pathlib import Path
+from typing import Optional
 from urllib.parse import quote_plus, urljoin
 
 import requests
@@ -784,7 +784,7 @@ def get_file_size(pathobj: Path):
 
 
 def get_checksum(pathobj: Path):
-    with open(pathobj,'rb') as file_to_check:
+    with open(pathobj, "rb") as file_to_check:
         # pipe contents of the file through
         return hashlib.md5(file_to_check.read()).hexdigest()
 

From e9bda9d1c4cf72eb6f69e63cffe3cbcc811433d7 Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Tue, 28 Nov 2023 10:47:42 +0100
Subject: [PATCH 5/8] Exclude _version.py from linting

Remove redundant black exclusions.
---
 pyproject.toml | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 63dd5b2..5ea50b7 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -58,26 +58,11 @@ docs = [
 
 [tool.black]
 include = '\.pyi?$'
-exclude = '''
-/(
-    \.git
-  | \.hg
-  | \.mypy_cache
-  | \.tox
-  | \.venv
-  | _build
-  | buck-out
-  | build
-  | dist
-
-  # The following are specific to Black, you probably don't want those.
-  | blib2to3
-  | tests/data
-)/
-'''
+extend_exclude = 'pyscicat/_version.py'
 
 [tool.isort]
 profile = "black"
+skip_glob = ["__pycache__", "pyscicat/_version.py"]
 
 [tool.hatch]
 version.source = "vcs"
@@ -85,4 +70,4 @@ build.hooks.vcs.version-file = "pyscicat/_version.py"
 
 [tool.mypy]
 # TODO Add type annotations and include this file
-exclude = "pyscicat/hdf5/scientific_metadata.py"
\ No newline at end of file
+exclude = ["pyscicat/_version.py", "pyscicat/hdf5/scientific_metadata.py"]
\ No newline at end of file

From f5261c5787d25b61368cffbf1677923ad9da9f05 Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Tue, 28 Nov 2023 10:49:55 +0100
Subject: [PATCH 6/8] Add types-requests for mypy tests

---
 pyproject.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pyproject.toml b/pyproject.toml
index 5ea50b7..2144d35 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -43,6 +43,7 @@ dev = [
     "requests_mock",
     "isort",
     "mypy",
+    "types-requests",
 ]
 docs = [
     "ipython",

From 926d3edc001b8a1b532aaaf550354dd3904b6800 Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Thu, 30 Nov 2023 09:28:15 +0100
Subject: [PATCH 7/8] Reverted back to invalid None assignment

Update methods should really take partial models with all-optional
contents (#58). Rather than fixing that issue, let's just ignore
the typing error for now.
---
 pyscicat/client.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pyscicat/client.py b/pyscicat/client.py
index 4cdbe6a..14740a8 100644
--- a/pyscicat/client.py
+++ b/pyscicat/client.py
@@ -489,7 +489,8 @@ def proposals_update(
         if proposalId is None:
             assert proposal.proposalId is not None, "proposalId should not be None"
             proposalId = proposal.proposalId
-        proposal.proposalId = ""
+        # TODO updates should allow partial proposals, where all fields are optional. See #58
+        proposal.proposalId = None  # type: ignore [assignment]
         result = self._call_endpoint(
             cmd="patch",
             endpoint=f"Proposals/{quote_plus(proposalId)}",

From 73458bf80339f85c7ac830fbbd8e906b3abb9dab Mon Sep 17 00:00:00 2001
From: Spencer Bliven <spencer.bliven@gmail.com>
Date: Thu, 30 Nov 2023 09:36:34 +0100
Subject: [PATCH 8/8] Require Datablock.version. This matches the DTO.

---
 pyscicat/model.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pyscicat/model.py b/pyscicat/model.py
index 84c5a15..1fd2723 100644
--- a/pyscicat/model.py
+++ b/pyscicat/model.py
@@ -194,7 +194,7 @@ class Datablock(Ownable):
 
     packedSize: Optional[int] = None
     chkAlg: Optional[int] = None
-    version: str = ""
+    version: str
     instrumentGroup: Optional[str] = None
     datasetId: str