Skip to content

2823 enhance convert_data/dst_type typing#3749

Merged
wyli merged 52 commits intodevfrom
enhance-convert-typing
Feb 4, 2022
Merged

2823 enhance convert_data/dst_type typing#3749
wyli merged 52 commits intodevfrom
enhance-convert-typing

Conversation

@wyli
Copy link
Copy Markdown
Contributor

@wyli wyli commented Jan 31, 2022 •
edited
Loading

part of #2823, typing enhancement as part of #3701

Description

enhance the monai.utils.convert_data_type to have more precise typing (as it is used extensively across the modules):

from monai.utils.type_conversion import convert_data_type, convert_to_dst_type
from typing import Any

import torch
import numpy as np

a = torch.Tensor(10.0)
b = convert_to_dst_type(1.0, dst=a)[0]
reveal_type(b)

c = convert_data_type(a, np.ndarray)[0]
reveal_type(c)

d = convert_data_type(a, torch.Tensor)[0]
reveal_type(d)

e: Any = convert_data_type(a, None)[0]
reveal_type(e)

f = convert_data_type(a, type(a))[0]
reveal_type(f)

will reveal the expected types:

test.py:9:13: note: Revealed type is "torch._tensor.Tensor*"
test.py:12:13: note: Revealed type is "numpy.ndarray*[_ShapeType`1, _DType_co`2]"
test.py:15:13: note: Revealed type is "torch._tensor.Tensor*"
test.py:18:13: note: Revealed type is "Any"
test.py:21:13: note: Revealed type is "torch._tensor.Tensor*"

as a baseline, the current dev branch outcome is:

test.py:9:13: note: Revealed type is "Union[numpy.ndarray[Any, Any], torch._tensor.Tensor]"
test.py:12:13: note: Revealed type is "Union[numpy.ndarray[Any, Any], torch._tensor.Tensor]"
test.py:15:13: note: Revealed type is "Union[numpy.ndarray[Any, Any], torch._tensor.Tensor]"
test.py:18:13: note: Revealed type is "Any"
test.py:21:13: note: Revealed type is "Union[numpy.ndarray[Any, Any], torch._tensor.Tensor]"

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

wyli and others added 30 commits January 25, 2022 18:20
@wyli
temp spatial_resample
dad9365 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes resampling
0f07fc9 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes precisions
b09e568 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update dict version
0b4bc52 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes unit tests
996dacf 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
adds docs
2790ca4 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
copy grid for resampling
4ef11bf 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes unit tests
f65605e 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
remove normalize coordinates
b3011a7 
Signed-off-by: Wenqi Li <[email protected]>
@monai-bot @wyli
[MONAI] python code formatting
a1200dc 
Signed-off-by: monai-bot <[email protected]>
@wyli
try to fix #3621 (#3673)
7b91be4 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes typing
ec9aaf3 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes grid_sample, interpolate URLs
0ac197f 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge branch 'fixes-url' into add-spatial-resample
87ff30b 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge remote-tracking branch 'upstream/dev' into add-spatial-resample
080f803 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
simplify norm_coords
aee76ee 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update docstring
0dc88af 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update moveaxis
d044188 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
spatial sample tests
7a42452 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
additional tests spatial resample
5ec96a4 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
test invert saptial resample
6509070 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes unit tests
bc9c705 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge remote-tracking branch 'upstream/dev' into add-spatial-resample
47bf3e2 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
rtol assert close
da1bc43 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes TF32 tests
cb1f12e 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
smaller tests
9e17297 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
skip when quick testing
75c602e 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge remote-tracking branch 'upstream/dev' into add-spatial-resample
bdd9bc8 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
comp tensor and ndarray
dbaac52 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update based on comments
fafb9ee 
Signed-off-by: Wenqi Li <[email protected]>
@wyli wyli force-pushed the enhance-convert-typing branch from a568c0f to 1e56adc Compare February 3, 2022 11:18
@wyli
formatting
ac3187f 
Signed-off-by: Wenqi Li <[email protected]>
@wyli wyli force-pushed the enhance-convert-typing branch from 1e56adc to ac3187f Compare February 3, 2022 11:24
@wyli wyli marked this pull request as ready for review February 3, 2022 14:06
@wyli wyli requested review from Nic-Ma, ericspod and rijobro and removed request for Nic-Ma and rijobro February 3, 2022 14:06
wyli added 2 commits February 4, 2022 00:18
@wyli
update type def.
13a031f 
Signed-off-by: Wenqi Li <[email protected]>

Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge branch 'dev' into enhance-convert-typing
76fe529 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 4, 2022

/build

wyli added 6 commits February 4, 2022 10:09
@wyli
temp test
ddc4770 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
warn unused
e645807 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
remote ignore
17bb916 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Revert "warn unused"
a099ee0 
This reverts commit e645807.

Signed-off-by: Wenqi Li <[email protected]>
@wyli
Revert "temp test"
559dcb4 
This reverts commit ddc4770.

Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge remote-tracking branch 'upstream/dev' into enhance-convert-typing
673dca6 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 4, 2022

Hi @rijobro the main changes are in

MONAI/monai/utils/type_conversion.py

Line 213 in 673dca6

def convert_data_type(
, the rest of this pr is mainly to remove unused type ignore commands. NdarrayOrTensor is still the primary array type, because mypy is still not powerful enough to infer the detailed types in many cases. so this wouldn't close #2823

but the previous

img_np: np.ndarray
img_np, *_ = convert_data_type(image, np.ndarray)  # type: ignore

becomes

img_np, *_ = convert_data_type(image, np.ndarray)

Nic-Ma
Nic-Ma approved these changes Feb 4, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

Thanks for the enhancement.
Looks good to me, put some minor comments inline.
So we should always use NdarrayTensor in future development?

Thanks.

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 4, 2022

/build

Thanks for the enhancement. Looks good to me, put some minor comments inline. So we should always use NdarrayTensor in future development?

Thanks.

could try to use NdarrayTensor if it works, otherwise NdarrayOrTensor would be fine.

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 4, 2022

/build

@wyli wyli enabled auto-merge (squash) February 4, 2022 14:53
@wyli wyli merged commit 816f413 into dev Feb 4, 2022
@wyli wyli deleted the enhance-convert-typing branch February 4, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nic-Ma Nic-Ma Nic-Ma approved these changes

@ericspod ericspod Awaiting requested review from ericspod ericspod is a code owner

@rijobro rijobro Awaiting requested review from rijobro

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wyli @rijobro @Nic-Ma @monai-bot