Skip to content

Conversation

@Nic-Ma
Copy link
Contributor

@Nic-Ma Nic-Ma commented Jan 11, 2023

Description

This PR updated several modules tutorials to match the format requirements of contribution guideline.
CC @mingxin-zheng .

Checks

  • Avoid including large-size files in the PR.
  • Clean up long text outputs from code cells in the notebook.
  • For security purposes, please check the contents and remove any sensitive info such as user names and private key.
  • Ensure (1) hyperlinks and markdown anchors are working (2) use relative paths for tutorial repo files (3) put figure and graphs in the ./figure folder
  • Notebook runs automatically ./runner.sh -t <path to .ipynb file>

Signed-off-by: Nic Ma <nma@nvidia.com>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Signed-off-by: Nic Ma <nma@nvidia.com>
Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma Nic-Ma changed the title [WIP] Update modules notebooks to match format requirements Update modules notebooks to match format requirements Jan 11, 2023
@Nic-Ma Nic-Ma marked this pull request as ready for review January 11, 2023 16:05
@mingxin-zheng
Copy link
Contributor

In tutorials/modules/layer_wise_learning_rate.ipynb, the RSNA Bone Age Challenge link

http://rsnachallenges.cloudapp.net/competitions/4

isn't working for me.

@mingxin-zheng
Copy link
Contributor

In modules/image_dataset.ipynb, there is an extra . after seg in this line:

print("seg. shape:", img_dataset[0][1].shape)

@mingxin-zheng
Copy link
Contributor

In modules/integrate_3rd_party_transforms.ipynb, there is a pip install for itk:

!python -c "import itk" || pip install -q itk==5.1.0

Can it be merged with pip install monai as the following?

!python -c "import monai" || pip install -q "monai-weekly[nibabel, itk]"

Thanks

@mingxin-zheng
Copy link
Contributor

mingxin-zheng commented Jan 12, 2023

In modules/inverse_transforms_and_test_time_augmentations.ipynb, there are a few duplicated %matplotlib inline and %matplotlib inline (for example, under Display some examples)

@mingxin-zheng
Copy link
Contributor

In Appendix: layers of DenseNet 121 network of modules/layer_wise_learning_rate.ipynb, the network structure is printed out as text outputs. There are 621 lines and is this necessary to keep in the notebook?

@mingxin-zheng
Copy link
Contributor

In modules/mednist_GAN_tutorial.ipynb, the RSNA Bone Age Challenge link needs to be fixed. Also, the notebook has the following line and it seems a bit weird to me. Should it be https://medmnist.com/?

If you use the MedNIST dataset, please acknowledge the source, e.g.
https://github.com/Project-MONAI/tutorials/blob/main/2d_classification/mednist_tutorial.ipynb.

@mingxin-zheng
Copy link
Contributor

In modules/mednist_GAN_workflow_array.ipynb, the following line can use a relative path to the mednist_GAN_tutorial notebook:

"Read the [MONAI Mednist GAN Tutorial](https://github.com/Project-MONAI/tutorials/blob/main/modules/mednist_GAN_tutorial.ipynb) for details about the network architecture and loss functions.\n",

@mingxin-zheng
Copy link
Contributor

In modules/mednist_GAN_workflow_dict.ipynb, the following lines need to use relative path, or replace the link with external site (MedNIST.com)

Read the [MONAI Mednist GAN Tutorial](https://github.com/Project-MONAI/tutorials/blob/main/modules/mednist_GAN_tutorial.ipynb) for details about the network architecture and loss functions.
If you use the MedNIST dataset, please acknowledge the source, e.g.
https://github.com/Project-MONAI/tutorials/blob/main/2d_classification/mednist_tutorial.ipynb.

And the RSNA links is invalid

The MedNIST dataset was gathered from several sets from [TCIA](https://wiki.cancerimagingarchive.net/display/Public/Data+Usage+Policies+and+Restrictions), [the RSNA Bone Age Challenge](http://rsnachallenges.cloudapp.net/competitions/4), and [the NIH Chest X-ray dataset](https://cloud.google.com/healthcare/docs/resources/public-datasets/nih-chest).

Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jan 12, 2023

Hi @mingxin-zheng ,

Thanks for your suggestion, I think we need fixed version of ITK in this tutorial, so I didn't change it.

Signed-off-by: Nic Ma <nma@nvidia.com>
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jan 12, 2023

Hi @mingxin-zheng ,

I have updated the PR according to all your comments.

Thanks for the review.

@mingxin-zheng
Copy link
Contributor

Hi @Nic-Ma , can you try to merge the main branch and see if the relevant notebooks in the PR can pass copyright and guideline tests? Thanks!

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jan 12, 2023

Hi @Nic-Ma , can you try to merge the main branch and see if the relevant notebooks in the PR can pass copyright and guideline tests? Thanks!

I don't see error related to this PR.

Thanks.

@mingxin-zheng
Copy link
Contributor

Hi @Nic-Ma , thanks for the update. I think the pep8 test shows there is an import error in the jupyter_util.ipynb. Can you fix it when you have time?

@mingxin-zheng
Copy link
Contributor

mingxin-zheng commented Jan 16, 2023

@Nic-Ma a minor fix is needed here:

Running ./modules/jupyter_utils.ipynb
Checking PEP8 compliance...
stdin:45:1: F401 'monai' imported but unused
import monai

I don't see any other issues. Please feel free to merge after the fix if there is no more pep8 failures

@Nic-Ma Nic-Ma enabled auto-merge (squash) January 16, 2023 10:01
@Nic-Ma Nic-Ma merged commit b312fc8 into main Jan 16, 2023
@wyli wyli deleted the update-modules-toolkit branch January 30, 2023 09:40
boneseva pushed a commit to boneseva/MONAI-tutorials that referenced this pull request Apr 21, 2024
…1156)

### Description
This PR updated several modules tutorials to match the format
requirements of contribution guideline.
CC @mingxin-zheng .

### Checks
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Avoid including large-size files in the PR.
- [x] Clean up long text outputs from code cells in the notebook.
- [x] For security purposes, please check the contents and remove any
sensitive info such as user names and private key.
- [x] Ensure (1) hyperlinks and markdown anchors are working (2) use
relative paths for tutorial repo files (3) put figure and graphs in the
`./figure` folder
- [x] Notebook runs automatically `./runner.sh -t <path to .ipynb file>`

Signed-off-by: Nic Ma <nma@nvidia.com>
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.

5 participants