Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a mechanism for users to know the availability of cucim.CuImage #107

Merged
merged 2 commits into from
Sep 30, 2021
Merged

Add a mechanism for users to know the availability of cucim.CuImage #107

merged 2 commits into from
Sep 30, 2021

Conversation

gigony
Copy link
Contributor

@gigony gigony commented Sep 28, 2021 •
edited
Loading

  • Add cucim.is_available() method

Users can query if a specific module is available or not.

For example, if cucim.clara module which requires C shared library is not available:

>>> import cucim
>>> cucim.is_available()
False
>>> cucim.is_available("skimage")
True
>>> cucim.is_available("core")
True
>>> cucim.is_available("clara")
False

Resolves #104
Supports Project-MONAI/MONAI#2987

@gigony gigony added feature request New feature or request non-breaking Introduces a non-breaking change labels Sep 28, 2021
@gigony gigony added this to the v21.10 milestone Sep 28, 2021
@gigony gigony self-assigned this Sep 28, 2021
@gigony gigony requested a review from a team as a code owner September 28, 2021 21:49
@gigony gigony added this to PR-Needs review in v21.10 Release via automation Sep 28, 2021
grlee77
grlee77 approved these changes Sep 28, 2021
View reviewed changes
Copy link
Contributor

@grlee77 grlee77 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @gigony

v21.10 Release automation moved this from PR-Needs review to PR-Reviewer approved Sep 28, 2021
jakirkham
jakirkham reviewed Sep 29, 2021
View reviewed changes
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks Gigon! 😄

Looks good. Had one question above

python/cucim/src/cucim/__init__.py
except ImportError:
from ._version import get_versions
__version__ = get_versions()['version']
del get_versions
del _version


def is_available(module_name: str = "") -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a test for this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback!
Added unit tests in the latest patch.

jakirkham
jakirkham approved these changes Sep 29, 2021
View reviewed changes
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Gigon! 😄

@gigony
Copy link
Contributor Author

gigony commented Sep 29, 2021

@jakirkham Conda-related CI/CD failure happens again here.

@jakirkham
Copy link
Member

Are we sure it is Conda related? Looks like it happens part way through the build. Please see snippet below:

15:35:06 [ 77%] Building CXX object CMakeFiles/cucim.dir/pybind11/filesystem/cufile_py.cpp.o

https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci/job/cucim/job/prb/job/cucim-cpu-python-build/CUDA=11.0,PYTHON=3.7/61/console

@jakirkham
Copy link
Member

Would recommend increasing verbosity from cmake & make to get a better idea on why the build dies there

@gigony
Copy link
Contributor Author

gigony commented Sep 29, 2021

Would recommend increasing verbosity from cmake & make to get a better idea on why the build dies there

#106

https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci/job/cucim/job/prb/job/cucim-cpu-python-build/CUDA=11.2,PYTHON=3.7/61/console

Looks like the failure is because #106 is merged.

LINE 1123

15:34:17 /workspace/.conda-bld/cucim_1632954074303/work/python/pybind11/cucim_py.cpp: In function 'void cucim::pybind11_init__cucim(pybind11::module_&)':
15:34:17 /workspace/.conda-bld/cucim_1632954074303/work/python/pybind11/cucim_py.cpp:156:33: error: 'close' is not a member of 'cucim::CuImage'
15:34:17   156 |         .def("close", &CuImage::close, doc::CuImage::doc_close, py::call_guard<py::gil_scoped_release>()) //
15:34:17       |                                 ^~~~~
15:34:17 /workspace/.conda-bld/cucim_1632954074303/work/python/pybind11/cucim_py.cpp: In lambda function:
15:34:17 /workspace/.conda-bld/cucim_1632954074303/work/python/pybind11/cucim_py.cpp:168:24: error: 'using element_type = class cucim::CuImage' {aka 'class cucim::CuImage'} has no member named 'close'
15:34:17   168 |                 cuimg->close(); //
15:34:17       |                        ^~~~~
15:34:21 gmake[3]: *** [CMakeFiles/cucim.dir/pybind11/cucim_py.cpp.o] Error 1

Will rebase this on top of the latest change.

@jakirkham
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 21d08ee into rapidsai:branch-21.10 Sep 30, 2021
v21.10 Release automation moved this from PR-Reviewer approved to Done Sep 30, 2021
@gigony gigony changed the title Add a mechanism for user to know the availability of cucim.CuImage Add a mechanism for users to know the availability of cucim.CuImage Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakirkham jakirkham jakirkham approved these changes

@grlee77 grlee77 grlee77 approved these changes

@quasiben quasiben Awaiting requested review from quasiben

Assignees

@gigony gigony

Labels
feature request New feature or request non-breaking Introduces a non-breaking change
Projects
No open projects
v21.10 Release
Done
Milestone
v21.10
Development

Successfully merging this pull request may close these issues.

[BUG] There is no mechanism for user to know the availability of cucim.CuImage
3 participants
@gigony @jakirkham @grlee77