Add ICT to CLT conversion #271
Conversation
JesseMckinzie
commented
Sep 10, 2024
JesseMckinzie
commented
- Add function to convert ICT to CLT
- Add ICT spec code
There was a problem hiding this comment.
This whole thing isn't implemented at all. Do we need to include in our repo?
There was a problem hiding this comment.
This is implemented and is needed for casting different types to the ICT object. See the singledispatch function on line 12 with corresponding functions for different argument types on lines 19, 37, and 55.
There was a problem hiding this comment.
Pretty sure this isn't needed as this generates semver for the ict_spec repo.
src/sophios/api/utils/converter.py
Outdated
| dict: A dictionary containing the CLT | ||
| """ | ||
|
|
||
| ict_local = ict if isinstance(ict, ICT) else validate(ict) |
There was a problem hiding this comment.
validate isn't implemented, just nuke it and raise an error here. Also see if you can eliminate some of the bloat and boiler plate.
There was a problem hiding this comment.
Validate is implemented and needed for casting the different types of arguments to the ICT object
tests/test_ict_to_clt_conversion.py
Outdated
There was a problem hiding this comment.
This is needs to redone. Follow the pattern of existing test pattern (See test_rest_core.py and test_examples.py). Also you need to add command to run these tests in runworkflows.yml so that these become part of CI.
tests/data/czi_extract.py
Outdated
There was a problem hiding this comment.
Better if all these data objects were .json files as these aren't code. Need more descriptive folder name than just 'data', maybe 'ict_clt_data'
There was a problem hiding this comment.
This is just an aesthetic thing. You should format these data files cmd+k cmd+f. Or write them out with correct indent when generating them.
JesseMckinzie
force-pushed
the
ict_to_clt_conversion
branch
from
b69bb85 to
61ac1a0
Compare
|
closed in favor of #275 |
