Skip to content

Add domain classes for study selected series and selection operator #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Nov 17, 2021

Conversation

MMelQin
Copy link
Collaborator

@MMelQin MMelQin commented Nov 11, 2021

Adding domain classes SeletedSeries and StudySelectedSeries, with the former encapsulate the selection name, DICOMSeries, and Image if applicable. The SelectedSeries are contained as a list in a DICOMStudy.
Adding simple selection rules in JSON, matching on DICOM Study and Series module attributes, limited to those exposed by DICOMStudy and DICOMSeries.
Adding selection logic in the DICOMSeriesSelection operator, as well as changing the output to list of StudySelectedSeries objects. It is extendable, and custom operators can be chained.
Update the DICOMSeriesToVolumeOperator to take in list of StudySelectedSeries object, and convert all series, though still limited to output the first series' image, as the inference operator can only support one input image.
Update operators' test methods.
Updated applicable example applications and Jupyter notebook to use the new API.
Updated domain classes to comply with DICOM Keywords for properties.
Updated DICOM loading operator to set domain objects' properties correctly.

Note: the style checker in the build/test fails because it complains about the camel case of the DICOM keyword properties of the DICOMStudy and DICOMSeries class, but it is best to use DICOM Standards based keywords names than the Python preferred naming convention (Pydicom also uses DICOM keywords as properties, so it is also consistent)

@MMelQin MMelQin force-pushed the mqin/add_rule_based_series_selection branch from 6490e09 to a1eb2d0 Compare November 15, 2021 06:49
Copy link
Collaborator

@gigony gigony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!
Looks good to me.
Please commit with a signature and fix style check failures.

@MMelQin
Copy link
Collaborator Author

MMelQin commented Nov 16, 2021

@gigony thanks for your input. Will make changes soon and push.

@MMelQin MMelQin force-pushed the mqin/add_rule_based_series_selection branch from 77202a8 to 6920d20 Compare November 16, 2021 00:43
@MMelQin MMelQin changed the title [WIP] Add domain classes for study selected series and selection operator Add domain classes for study selected series and selection operator Nov 16, 2021
@MMelQin
Copy link
Collaborator Author

MMelQin commented Nov 16, 2021

I ran ./run check --autofix, python3 -m mypy --namespace-packages --explicit-package-bases . , and python3 -m flake8 . --count --statistics without any errors, before pushing the commit.

But getting the error in the build check
021-11-16 04:56:13 $ python3 -m flake8 /home/runner/work/monai-deploy-app-sdk/monai-deploy-app-sdk --count --statistics /home/runner/work/monai-deploy-app-sdk/monai-deploy-app-sdk/monai/deploy/operators/dicom_series_selector_operator.py:16:1: F401 'typing.Any' imported but unused 1 F401 'typing.Any' imported but unused

Oh well.

@MMelQin MMelQin force-pushed the mqin/add_rule_based_series_selection branch from 37222bc to bec2cd4 Compare November 16, 2021 05:14
1 file reformatted, 113 files left unchanged

Signed-off-by: mmelqin <[email protected]>
Signed-off-by: mmelqin <[email protected]>
Signed-off-by: mmelqin <[email protected]>
Signed-off-by: mmelqin <[email protected]>
Signed-off-by: mmelqin <[email protected]>
@MMelQin MMelQin merged commit a1d683d into main Nov 17, 2021
@gigony gigony mentioned this pull request Nov 24, 2021
@MMelQin MMelQin deleted the mqin/add_rule_based_series_selection branch February 3, 2025 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants