Skip to content

Fixed dependency issues with itkwidgets #1845

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

Closed
wants to merge 2 commits into from

Conversation

kirbyju
Copy link
Contributor

@kirbyju kirbyju commented Sep 24, 2024

Fixes # .

Description

Fixed the notebook to address InsightSoftwareConsortium/itkwidgets#763.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

review-notebook-app bot commented Sep 25, 2024

View / edit / reply to this conversation on ReviewNB

KumoLiu commented on 2024-09-25T06:21:04Z
----------------------------------------------------------------

Please also remove the output for this cell, thanks.


Copy link

review-notebook-app bot commented Sep 25, 2024

View / edit / reply to this conversation on ReviewNB

KumoLiu commented on 2024-09-25T06:21:05Z
----------------------------------------------------------------

Line #13.    !{sys.executable} -m pip install -q opencv-python-headless

Could you please revert the change here, first check whether the package can be imported successfully, if not then install? Otherwise the premerge will fail since we already have opencv installed if install another version which will cause issue. Thanks.


@KumoLiu
Copy link
Contributor

KumoLiu commented Sep 25, 2024

Hi @kirbyju, thanks for the pr. It's wired it seems you changed all the notebook, could you please only submit the necessary update instead of overwrite all the things? It may not easy to track the difference, thanks.
And BTW, could you please sigh off the commit to fix the DCO issue: https://github.com/Project-MONAI/tutorials/pull/1845/checks?check_run_id=30601781640
And Also you may set the change in a non-protected branch, seems the pre-commit can not push the change to your main(protected) branch: https://results.pre-commit.ci/run/github/288774122/1727200995.8EnmrDQBS5CsvdsH56r4Sw
Thanks again.

@KumoLiu KumoLiu requested review from ericspod and Nic-Ma September 25, 2024 06:25
@kirbyju
Copy link
Contributor Author

kirbyju commented Oct 3, 2024

Hi @KumoLiu , I'm going to close this PR and submit a new one in a separate branch that will hopefully also address the issue of it showing everything as "new" instead of only showing the modified lines.

@kirbyju kirbyju closed this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants