minor updates/fixes for consistency with scikit-image 0.22 by grlee77 · Pull Request #670 · rapidsai/cucim

grlee77 · 2024-01-06T18:42:19Z

This MR does not add any new features. It just has minor fixes ported from scikit-image 0.22. Most changed lines are from moving additional modules to use __init__.pyi files as for upstream scikit-image.

Regarding introduction of py.typed, see discussion on the scikit-image repo. The TLDR version is that it enables VS CODE (and potentially other IDEs) to use the .pyi files for intellisense capabilities. Those using mypy with strict type checking, may have to add an exclusion like the following to disable warnings about most of the cuCIM API which does not currently use type hints:

[tool.mypy]
strict = true
untyped_calls_exclude = [
    "cucim"
]

gigony

Thank you, @grlee77, for this update. It's helpful to learn about the method for making the document accessible in the IDE, as shown in this pull request.
Looks good to me!

jakirkham · 2024-01-18T22:25:53Z

It looks like the style checks are failing on CI. The single quotes need to be replaced with double quotes. There are also some long lines that need to be wrapped

Could you please run black @grlee77 ?

This is the default (and for CuPy it is the only implemented option). scikit-image uses numpy with kind='stable'

…r gaussian_filter

…pends on mask size see: scikit-image/scikit-image#7136

… properties exist in COL_DTYPES

…return_error argument in phase_cross_correlation

np.NAN -> np.nan

needed in order for the .pyi files to be useful: see: scikit-image/scikit-image#7073

grlee77 · 2024-01-23T21:18:07Z

The regionprops test failure was a due to a couple missing entries in PROPS and should be fixed now.

I didn't see that jitify error related to std::is_signed when trying locally with pip packages, but conda seems to have a problem again with that now.

gigony

Thanks @grlee77
The updated portion of the changes looks good to me!

jakirkham

Thanks Greg! 🙏

Had a few comments below

python/cucim/src/cucim/skimage/_shared/filters.py

python/cucim/src/cucim/skimage/_vendored/_internal.py

python/cucim/src/cucim/skimage/color/colorconv.py

python/cucim/src/cucim/skimage/color/tests/test_colorconv.py

python/cucim/src/cucim/skimage/filters/ridges.py

python/cucim/src/cucim/skimage/measure/_blur_effect.py

python/cucim/src/cucim/skimage/restoration/deconvolution.py

python/cucim/src/cucim/skimage/restoration/j_invariant.py

python/cucim/src/cucim/skimage/transform/_geometric.py

python/cucim/src/cucim/skimage/util/dtype.py

Co-authored-by: jakirkham <jakirkham@gmail.com>

jakirkham

Thanks Greg! 🙏

jakirkham · 2024-01-26T01:28:03Z

As pointed out by Greg offline, we need to update this line and run rapids-dependency-file-generator

cucim/dependencies.yaml

Line 226 in bf5a6f7

- scikit-image>=0.19.0,<0.22.0a0

jakirkham

Need to update scikit-image upper bound

python/cucim/requirements-test.txt

Resolved

jakirkham · 2024-01-26T22:13:09Z

Do we need PyWavelets as a (test) dependency?

Seeing these errors on CI:

FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_denoise_invariant_color[float16] - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.
FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_denoise_invariant_color[float32] - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.
FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_denoise_invariant_color[float64] - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.
FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_denoise_invariant_3d - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.
FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_calibrate_denoiser_extra_output - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.
FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_calibrate_denoiser - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.
FAILED python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py::test_input_image_not_modified - ImportError: PyWavelets is not installed. Please ensure it is installed in order to use this function.

jakirkham · 2024-01-27T01:38:44Z

Looks like this is used as part of denoise_wavelet, which we are using in these tests

As pointed out by Greg offline, upstream appears to have relaxed the PyWavelets dependency in 0.22.0 with PR ( scikit-image/scikit-image#7156 ), which is why the dependency is now missing

…available add pywavelets to test dependencies

jakirkham

Thanks Greg! 🙏

Think we might want to drop this top-level import and rewrite with something like this

python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py

Co-authored-by: jakirkham <jakirkham@gmail.com>

raydouglass

ops-codeowners changes look good

grlee77 · 2024-01-29T18:07:02Z

/merge

jakirkham · 2024-01-29T19:03:57Z

Thanks Greg! 🙏

grlee77 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change maintenance labels Jan 6, 2024

grlee77 added this to the v24.02.00 milestone Jan 6, 2024

grlee77 requested a review from a team as a code owner January 6, 2024 18:42

gigony approved these changes Jan 10, 2024

View reviewed changes

grlee77 added 22 commits January 22, 2024 14:31

try/except for importing AxisError from NumPy

028a6bb

be explicit about kind='stable'

33edf40

This is the default (and for CuPy it is the only implemented option). scikit-image uses numpy with kind='stable'

use an __init__.pyi for cucim.skimage.feature

babb859

update docstrings of ridge filters

cde479e

update threshold_triangle to handle constant-valued images

cda6a5b

complete deprecation cycle for switch to channel_axis=None default fo…

aabf4b7

…r gaussian_filter

skip call to pad in block_reduce when possible

d6cd721

implement fix for user-defined regionprop with extra property that de…

edb0ba0

…pends on mask size see: scikit-image/scikit-image#7136

fix missing entries in COL_DTYPES and test that all documented region…

905f557

… properties exist in COL_DTYPES

add __init__.pyi for skimage.measure

cbef585

add top-level docstring to morphology/__init__.py

127ed28

use __init__.pyi for skimage.registration module

cee53c2

remove support for return_error=False and deprecate specification of …

7adf7aa

…return_error argument in phase_cross_correlation

use __init__.pyi in skimage.restoration

1ac15f2

docstring updates to skimage.restoration

5826cde

np.NAN -> np.nan

add module level docstring to cucim.skimage.segmentation

abcba03

use __init__.pyi in skimage.transform

e5f32b4

update docstrings for geometric transforms

6f678d1

import style fix in pyramids.py

d12d6be

update dtype utility for future NumPy compatibility

067fe1a

Include py.typed file in distribution (PEP 561)

64071ba

needed in order for the .pyi files to be useful: see: scikit-image/scikit-image#7073

apply black to new __init__.pyi files

695121e

grlee77 force-pushed the grelee/skimage022-updates branch from d04791d to 695121e Compare January 22, 2024 19:35

add missing function to measure/__init__.pyi

41a5325

grlee77 force-pushed the grelee/skimage022-updates branch from b83c620 to 92db3c8 Compare January 23, 2024 19:46

grlee77 force-pushed the grelee/skimage022-updates branch from 92db3c8 to 41a5325 Compare January 24, 2024 00:41

Merge branch 'branch-24.02' into grelee/skimage022-updates

c4c9fc5

jakirkham removed the request for review from gigony January 25, 2024 19:19

gigony approved these changes Jan 25, 2024

View reviewed changes

jakirkham reviewed Jan 25, 2024

View reviewed changes

Apply suggestions from code review

0cc75e2

Co-authored-by: jakirkham <jakirkham@gmail.com>

jakirkham approved these changes Jan 26, 2024

View reviewed changes

jakirkham previously requested changes Jan 26, 2024

View reviewed changes

jakirkham reviewed Jan 26, 2024

View reviewed changes

python/cucim/requirements-test.txt Outdated Show resolved Hide resolved

jakirkham added 2 commits January 26, 2024 11:58

Merge rapidsai/branch-24.02 into grlee77/grelee/skimage022-updates

df17fc1

Bump scikit-image to 0.22

9891f89

grlee77 requested a review from a team as a code owner January 26, 2024 20:00

Add numpydoc constraint & include in tests

b886417

jakirkham force-pushed the grelee/skimage022-updates branch from 1024fea to b886417 Compare January 26, 2024 20:13

Merge branch 'branch-24.02' into grelee/skimage022-updates

7dd1fcf

skip skimage.restoration.test_j_invariant cases if pywavelets is not …

c0607ee

…available add pywavelets to test dependencies

jakirkham reviewed Jan 27, 2024

View reviewed changes

python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py Outdated Show resolved Hide resolved

python/cucim/src/cucim/skimage/restoration/tests/test_j_invariant.py Outdated Show resolved Hide resolved

Apply suggestions from code review

af56477

Co-authored-by: jakirkham <jakirkham@gmail.com>

jakirkham approved these changes Jan 28, 2024

View reviewed changes

raydouglass approved these changes Jan 29, 2024

View reviewed changes

rapids-bot bot merged commit caade43 into rapidsai:branch-24.02 Jan 29, 2024

Conversation

grlee77 commented Jan 6, 2024

Uh oh!

gigony left a comment

Choose a reason for hiding this comment

Uh oh!

jakirkham commented Jan 18, 2024

Uh oh!

grlee77 commented Jan 23, 2024

Uh oh!

gigony left a comment

Choose a reason for hiding this comment

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

jakirkham commented Jan 26, 2024

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakirkham commented Jan 26, 2024

Uh oh!

jakirkham commented Jan 27, 2024

Uh oh!

jakirkham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

raydouglass left a comment

Choose a reason for hiding this comment

Uh oh!

grlee77 commented Jan 29, 2024

Uh oh!

jakirkham commented Jan 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants