Skip to content

Conversation

@mingxin-zheng
Copy link
Contributor

@mingxin-zheng mingxin-zheng commented Dec 26, 2022

Signed-off-by: Mingxin Zheng 18563433+mingxin-zheng@users.noreply.github.com

Fixes #1119 .

Description

Provide guidelines for tutorial contributions

Checks

  • Notebook runs automatically ./runner [-p <regex_pattern>]

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

pre-commit-ci bot and others added 6 commits December 27, 2022 08:52
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@mingxin-zheng mingxin-zheng changed the title [WIP] Contributing guidelines Contributing guidelines Dec 29, 2022
@mingxin-zheng mingxin-zheng requested a review from Nic-Ma December 29, 2022 08:19
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@Nic-Ma Nic-Ma requested a review from wyli December 29, 2022 14:51
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Copy link
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

Thanks for the quick update, looks good to me.
@wyli @ericspod @rijobro , could you please help also review it?

Thanks in advance.

@Nic-Ma Nic-Ma requested review from ericspod and rijobro December 30, 2022 03:04
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Copy link
Member

@ericspod ericspod left a comment

Choose a reason for hiding this comment

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

Looks good to me!

mingxin-zheng and others added 3 commits January 3, 2023 17:26
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

the command of running one notebook appears multiple times ./runner.sh -p "-and -wholename , perhaps we add a new option ./runner.sh -t in the bash script as a shortcut?

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@mingxin-zheng
Copy link
Contributor Author

mingxin-zheng commented Jan 5, 2023

the command of running one notebook appears multiple times ./runner.sh -p "-and -wholename , perhaps we add a new option ./runner.sh -t in the bash script as a shortcut?

Agree and only a small question: is -t for --test? @wyli

Added the following in runner.sh

-t|--test)
	pattern+="-and -wholename ./$2"
	echo $pattern
	shift

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Jan 5, 2023

Thanks @mingxin-zheng The script changes look good

@mingxin-zheng
Copy link
Contributor Author

@wyli shall we change the Note to Developers in the frontpage README.md to something like "please refer to our contributing guideline"?

https://github.com/Project-MONAI/tutorials/blob/main/README.md#3-note-to-developers

In addition, the string to replace list in the note seems outdated. Maybe I can update that in this PR.

@wyli
Copy link
Contributor

wyli commented Jan 5, 2023

Sure, that'll be helpful. The pull request template could also be updated to remind the developers https://github.com/Project-MONAI/tutorials/blob/main/.github/pull_request_template.md

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@mingxin-zheng
Copy link
Contributor Author

Thanks @wyli . I added the following to the template, if that's what you are thinking of:

- [ ] I have read the [Contributing Guidelines](../CONTRIBUTING.md)

@wyli
Copy link
Contributor

wyli commented Jan 5, 2023

I think a URL to the contrib guide should be enough. But the template checklist should include more detailed items, like whether a license header is included, and maybe things you mentioned in the common recommendation section, file size, dataset license. The goal is to ask the contributors to do some basic self check for every pull request.

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
@mingxin-zheng
Copy link
Contributor Author

Thanks @wyli for the clarification, and here it goes:

- [ ] Avoid including large-size files in the PR.
- [ ] Clean up long text outputs from code cells in the notebook.
- [ ] Remove private info such as user name and private key from the notebook and script files.
- [ ] 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>`

@wyli wyli merged commit 8abec25 into Project-MONAI:main Jan 5, 2023
@mingxin-zheng mingxin-zheng deleted the guideline-1119 branch January 17, 2023 04:00
boneseva pushed a commit to boneseva/MONAI-tutorials that referenced this pull request Apr 21, 2024
Signed-off-by: Mingxin Zheng
<18563433+mingxin-zheng@users.noreply.github.com>

Fixes Project-MONAI#1119 .

### Description
Provide guidelines for tutorial contributions

### Checks
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Notebook runs automatically `./runner [-p <regex_pattern>]`

Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.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.

Contribution Guideline

4 participants