From ea72b6c52210b3e57eb2785bed9b000581e6908b Mon Sep 17 00:00:00 2001 From: John Wilkie <124276291+JBWilkie@users.noreply.github.com> Date: Thu, 7 Nov 2024 13:17:08 +0000 Subject: [PATCH] [DAR-4672][External] Replaced `no_legacy` with `legacy` & swapped default scaling behaviour of `NifTI` importer (#958) * Replaced with * Test coverage --- darwin/cli_functions.py | 15 +- darwin/importer/importer.py | 10 +- darwin/options.py | 21 +- .../data/nifti/BRAINIX_NIFTI_ROI.nii.gz | Bin 0 -> 5279 bytes .../data/nifti/legacy/.v7/metadata.json | 44 + .../nifti/legacy/BRAINIX_NIFTI_ROI.nii.json | 3949 +++++++++++++++++ tests/darwin/data/nifti/nifti.json | 12 + .../data/nifti/no-legacy/.v7/metadata.json | 44 + .../no-legacy/BRAINIX_NIFTI_ROI.nii.json | 3913 ++++++++++++++++ .../importer/formats/import_nifti_test.py | 106 + tests/darwin/importer/importer_test.py | 8 + 11 files changed, 8099 insertions(+), 23 deletions(-) create mode 100644 tests/darwin/data/nifti/BRAINIX_NIFTI_ROI.nii.gz create mode 100644 tests/darwin/data/nifti/legacy/.v7/metadata.json create mode 100644 tests/darwin/data/nifti/legacy/BRAINIX_NIFTI_ROI.nii.json create mode 100644 tests/darwin/data/nifti/nifti.json create mode 100644 tests/darwin/data/nifti/no-legacy/.v7/metadata.json create mode 100644 tests/darwin/data/nifti/no-legacy/BRAINIX_NIFTI_ROI.nii.json diff --git a/darwin/cli_functions.py b/darwin/cli_functions.py index 32f996366..8ff145054 100644 --- a/darwin/cli_functions.py +++ b/darwin/cli_functions.py @@ -919,8 +919,8 @@ def dataset_import( If ``True`` it will bypass a warning that the import will overwrite the current annotations if any are present. If ``False`` this warning will be skipped and the import will overwrite the current annotations without warning. legacy : bool, default: False - If ``True`` it will not resize the annotations to be isotropic. - If ``False`` it will resize the annotations to be isotropic. + If ``True`` it will resize the annotations to be isotropic. + If ``False`` it will not resize the annotations to be isotropic. use_multi_cpu : bool, default: False If ``True`` it will use all multiple CPUs to speed up the import process. cpu_limit : Optional[int], default: Core count - 2 @@ -931,7 +931,6 @@ def dataset_import( try: importer: ImportParser = get_importer(format) - if format == "nifti" and legacy: importer = partial(importer, legacy=True) @@ -954,7 +953,7 @@ def dataset_import( overwrite, use_multi_cpu, cpu_limit, - no_legacy=False if legacy else True, + legacy, ) except ImporterNotFoundError: @@ -1228,8 +1227,8 @@ def dataset_convert( annotations folder of the dataset under 'other_formats/{format}'. legacy : bool, default: False This flag is only for the nifti format. - If True, it will not export the annotations using legacy calculations. - If False, it will resize the annotations using the new calculation by dividing with pixdims. + If True, it will resize the annotations by dividing by pixdims. + If False, it will not export the annotations using legacy calculations """ identifier: DatasetIdentifier = DatasetIdentifier.parse(dataset_identifier) client: Client = _load_client(team_slug=identifier.team_slug) @@ -1286,8 +1285,8 @@ def convert( Folder where the exported annotations will be placed. legacy: bool, default: False This flag is only for the nifti format. - If True, it will not export the annotations using legacy calculations. - If False, it will resize the annotations using the new calculation by dividing with pixdims. + If True, it will resize the annotations by dividing by pixdims + If False, it will not export the annotations using legacy calculations. """ try: parser: ExportParser = get_exporter(format) diff --git a/darwin/importer/importer.py b/darwin/importer/importer.py index aad713e0c..ff1a06333 100644 --- a/darwin/importer/importer.py +++ b/darwin/importer/importer.py @@ -1092,7 +1092,7 @@ def import_annotations( # noqa: C901 overwrite: bool = False, use_multi_cpu: bool = False, cpu_limit: Optional[int] = None, - no_legacy: Optional[bool] = False, + legacy: Optional[bool] = False, ) -> None: """ Imports the given given Annotations into the given Dataset. @@ -1134,9 +1134,9 @@ def import_annotations( # noqa: C901 If ``cpu_limit`` is greater than the number of available CPU cores, it will be set to the number of available cores. If ``cpu_limit`` is less than 1, it will be set to CPU count - 2. If ``cpu_limit`` is omitted, it will be set to CPU count - 2. - no_legacy : bool, default: False - If ``True`` will not use the legacy isotropic transformation to resize annotations - If ``False`` will use the legacy isotropic transformation to resize annotations + legacy : bool, default: False + If ``True`` will use the legacy isotropic transformation to resize annotations + If ``False`` will not use the legacy isotropic transformation to resize annotations Raises ------- ValueError @@ -1154,7 +1154,7 @@ def import_annotations( # noqa: C901 # CLI-initiated imports will raise an AttributeError because of the partial function # This block handles SDK-initiated imports try: - if importer.__module__ == "darwin.importer.formats.nifti" and not no_legacy: + if importer.__module__ == "darwin.importer.formats.nifti" and legacy: importer = partial(importer, legacy=True) except AttributeError: pass diff --git a/darwin/options.py b/darwin/options.py index 6ac6c6717..7a59f137d 100644 --- a/darwin/options.py +++ b/darwin/options.py @@ -61,10 +61,10 @@ def __init__(self) -> None: help="Annotation files (or folders) to convert.", ) parser_convert.add_argument( - "--no-legacy", - action="store_false", - dest="legacy", - help="Do not convert annotation using legacy process (isotropic transformation).", + "--legacy", + action="store_true", + default=False, + help="Import annotation files using legacy process (isotropic transformation).", ) parser_convert.add_argument( "output_dir", type=str, help="Where to store output files." @@ -375,10 +375,10 @@ def __init__(self) -> None: help="Bypass warnings about overwiting existing annotations.", ) parser_import.add_argument( - "--no-legacy", - action="store_false", - dest="legacy", - help="Do not importing annotation files using legacy process (isotropic transformation).", + "--legacy", + action="store_true", + default=False, + help="Import annotation files using legacy process (isotropic transformation).", ) # Cpu limit for multiprocessing tasks @@ -410,9 +410,10 @@ def cpu_default_types(input: Any) -> Optional[int]: # type: ignore "format", type=str, help="Annotation format to convert to." ) parser_convert.add_argument( - "legacy", + "--legacy", action="store_true", - help="Convert annotation using legacy process (isotropic transformation).", + default=False, + help="Import annotation files using legacy process (isotropic transformation).", ) parser_convert.add_argument( "-o", "--output_dir", type=str, help="Where to store output files." diff --git a/tests/darwin/data/nifti/BRAINIX_NIFTI_ROI.nii.gz b/tests/darwin/data/nifti/BRAINIX_NIFTI_ROI.nii.gz new file mode 100644 index 0000000000000000000000000000000000000000..0eef75055b936f919c9dd0226c37bb44528d441f GIT binary patch literal 5279 zcmeHJ`!}0;9yW6(r&Fe)=yaLkrR^!EHO{KKY#S*tWLhn8DRs+CN4M0ysBSc`scA)3 z8+L~>N=T?snxa9~WzWV@U5W_OafzyX(y%HK5^wDHHD=HJ2hN`3{o#2(NzV6@=Xt)* z=L_BaRO7ctI*Pkn?Pu_E0x7mV?_^qEzJIFJz0dsy??Cux7T&*VrmlTF{OLP8o%Rmr zGg_FvQ!Y+A{r+zP!ZNa)kFIMhy{|i#pWniGAg|gmT@|tl* z(a$N8*M1@J`s&hyBKRalQow1Jt;$wU&^(Nc&(a_5#|mCbo_IhZ*H;u+#`>BnCddfy z7=K;CSC=NqE$V9CgpiC!Hu8ImsDJ0V&0#w1 zGI_1*1$0B|hTjk};H8=5lyzy6==z7-DeE(e#T40dKRO*^Y&2lWZOZ?Ak&+@`eHgQ$ zx=dF0dbFx&kCTd;uia3u&8kKF$x0~Y_!d7quN?+<7}#N8hk+diwvB;*E{8Yy_bt>y zd-&0<3@*lQMZq$S7GQTHS$$Y!G-XcUcb{@a!g0mTrNP6dC5F0UQx>GFL}B4n=*qs* z13mM!%+$^~8ax9=^~J>CloPh-XfOpIYk*{tu+)bkviRh~CC17qA=4-~s@G8Zv{YWz zkBuKqB!XUkN%IXic{P>3DLO(NqXFN|!c zVSOLo(87`QOC!k~D3|-b70wgZZz#VPAww(%Z2|VMLQY0ns)`0!#Wbl}!tW*6ewbnw zk>sA)6e63}esz!d`vRCaC7&yQvRB~dlxR8llw*wea2xb6aSvc(q+f|iF4qRYvw^o? z{_yGzMc?Q39DuHXk8o139Tgi4nxoxXt&F8%VX2)k%6%7;KN1baQCl&;k^x&3>{MhHP*L5jWy*^-ek19 zD?U`;rNhGYUrG6|*tDN1o#xxBS))z$QU8YX^i==4!TV|cLjm**`-TQ1OdJak7TZ5U zPbG>;dr|)?r-4w`z_IQ)WyNXyAsL~K1PErv0Vv7_U?}giig3Ym@8$hjSrO5_R?4V! z9gC593-Tz@j(UTcN75;-h2(+KE$^yS`P!oD_wqhdyEbH_;?S|$H%;auWnp`afz9XQ_6}d$HO{_&*XI+ldlZOI^0r>MR`-g z$2U>6GvUq(+AYeq9wUwGd5`Ka(jEyPEB#tR+FP9~;p+s}i%B{WG6J&UY(-j?@qLQw=0T*7 zv$*n^SrlTP{9g0zHJw3B|U#U9Wf95C6SLb9xH6T`MOa8Qwp%mijyD+27DZ=F7o z5uEWdZ?sG(kP+TCGp190I?B4>TyYqkt47ett zT=#1m$U*D2@J|03?le!>ytpVEM)vK^=yA7{iZh;(o`Ok92ym3@mIM4-WnLWdOEk z0DRtsgaJp(yK_n{qYoir?4}jU!`PsH)J}im(^JKp)#k;-6TO)H4sx{G1oW~3s!izE zc0jiOkN