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.

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

Quick fix for simplecrf imports #606

Merged

Conversation

masadcv
Copy link
Collaborator

@masadcv masadcv commented Feb 3, 2022

Signed-off-by: masadcv [email protected]

Work around for #605 until the error is debugged.

@masadcv
Copy link
Collaborator Author

masadcv commented Feb 3, 2022

@SachidanandAlle I made all SImpleCRF imports optional. Please give it a try by uninstalling SimpleCRF and running any app. On my end this exp runs fine.

@@ -31,6 +30,10 @@
maxflow3d,
)

densecrf, _ = optional_import("dennseCRF")
densecrf3d, _ = optional_import("denseCRF3D")
Copy link
Contributor

Choose a reason for hiding this comment

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

here, you could use:

Suggested change
densecrf3d, _ = optional_import("denseCRF3D")
densecrf3d, has_densecrf3d = optional_import("denseCRF3D")

and then you could use has_densecrf3d elsewhere to display an error if the user tries to use scribbles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks! will update this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good!

@SachidanandAlle
Copy link
Collaborator

SachidanandAlle commented Feb 4, 2022

This solves one part of the problem... when SimpleCRF is installed but import fails due to xyz reasons...
I guess there is another thread where installation itself can be simplified with pre-built binaries while installing SimpleCRF package

@SachidanandAlle SachidanandAlle added the 0.4.0 Targeted to Release version 0.4 label Feb 4, 2022
@SachidanandAlle SachidanandAlle linked an issue Feb 4, 2022 that may be closed by this pull request
@SachidanandAlle SachidanandAlle assigned masadcv and unassigned masadcv Feb 4, 2022
@SachidanandAlle SachidanandAlle merged commit 6cc72c5 into Project-MONAI:main Feb 4, 2022
@masadcv masadcv deleted the make-simplecrf-optionalimport branch February 5, 2022 19:45
Douwe-Spaanderman pushed a commit to Douwe-Spaanderman/MONAILabel that referenced this pull request Dec 9, 2022
* quick fix for simplecrf imports

Signed-off-by: masadcv <[email protected]>

* use has_import to give a nice ImportError

Signed-off-by: masadcv <[email protected]>

Co-authored-by: SACHIDANAND ALLE <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.4.0 Targeted to Release version 0.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

denseCRF3D import error
3 participants