search

SCIInstitute / shapeworks-cloud

A web version of ShapeWorks Studio
https://www.shapeworks-cloud.org/#/
Apache License 2.0
1 stars 0 forks source link

Update typing for server and swcc #356

Closed annehaley closed 9 months ago

annehaley commented 9 months ago

Resolves #351. This is related to the current failure on #355, which contains the following traceback:

type: commands[0]> mypy . --exclude=build/
/home/runner/work/shapeworks-cloud/shapeworks-cloud/swcc/.tox/type/lib/python3.8/site-packages/pydantic/__init__.py:45: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 0.910

Inevitably, with the update to mypy, the type coercion we had been performing for FileType objects in SWCC is no longer allowed, so this typing update will introduce breaking changes.

The SWCC ApiModel now separates File fields from their source fields, where the source field is typed as Union[str, Path]. For every file-type field on an SWCC class, a _source suffix should be added to the attribute when calling the constructor. For example, project = Project(file=file_path) becomes project = Project(file_source=file_path). After this constructor change has been made, project.file is still accessible and will return a File object with attributes path, url, field_id, and field_value. This accessor to the File object is generated dynamically in ApiModel.__getattribute__.

annehaley commented 9 months ago

@manthey Most of the typing changes in this PR were straightforward, but there was one variety of error that seemed to indicate a bigger problem. There are still 21 errors similar to the following: Argument "file" to "Project" has incompatible type "Path"; expected "FileType[Literal['core.Project.file']]

For every place that we passed a Path or a string to a field that was typed as FileType, this type error occured. We got away with this in the previous version, but the latest version strongly discourages changing the type of v in the validate function: https://github.com/pydantic/pydantic/discussions/3997

How do you think we should approach changing the way we set these fields?

annehaley commented 9 months ago

I was getting similar errors for NonEmptyString as for FieldType. The coercion we had been relying on was no longer accepted: 

 Argument "anatomy_type" to "Segmentation" has incompatible type "str"; expected "NonEmptyString"