Skip to content

Add Optuna and Improve Doc-strings for Auto3Dseg#5137

Merged
mingxin-zheng merged 10 commits intoProject-MONAI:devfrom
mingxin-zheng:optuna-yufan
Sep 14, 2022
Merged

Add Optuna and Improve Doc-strings for Auto3Dseg#5137
mingxin-zheng merged 10 commits intoProject-MONAI:devfrom
mingxin-zheng:optuna-yufan

Conversation

@mingxin-zheng
Copy link
Copy Markdown
Contributor

@mingxin-zheng mingxin-zheng commented Sep 13, 2022

Description

Improve docstring and Optuna Integration(#5136 )

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Nic-Ma
Nic-Ma reviewed Sep 13, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Hi @mingxin-zheng ,

Please add NNI and Optuna as optional dependency for the MONAI project, refer to:
https://github.com/Project-MONAI/MONAI/blob/dev/CONTRIBUTING.md#adding-new-optional-dependencies

Thanks.

fix commented items
81366c2 
Signed-off-by: Mingxin Zheng <[email protected]>
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Sep 13, 2022

Hi @wyli ,

I think we also need to add the optional dependency to this file:
https://github.com/Project-MONAI/MONAI/blob/dev/requirements-dev.txt
But the contribution guide doesn't include it, can @mingxin-zheng also update the contribution guide doc to include it?

Thanks.

Nic-Ma
Nic-Ma approved these changes Sep 13, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Looks good to me except for the:
https://github.com/Project-MONAI/MONAI/blob/dev/requirements-dev.txt

Thanks.

@mingxin-zheng
Copy link
Copy Markdown
Contributor Author

mingxin-zheng commented Sep 13, 2022

Looks good to me except for the: https://github.com/Project-MONAI/MONAI/blob/dev/requirements-dev.txt

Thanks.

Thanks @Nic-Ma . I have updated the template package and the requirement-dev as well.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Sep 13, 2022

/build

@mingxin-zheng mingxin-zheng enabled auto-merge (squash) September 14, 2022 01:22
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Sep 14, 2022

/build

@mingxin-zheng mingxin-zheng merged commit 50c30c1 into Project-MONAI:dev Sep 14, 2022
@Nic-Ma Nic-Ma mentioned this pull request Sep 14, 2022
Merged
7 tasks
@wyli wyli mentioned this pull request Sep 15, 2022
Closed
7 tasks
@mingxin-zheng mingxin-zheng deleted the optuna-yufan branch September 29, 2022 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nic-Ma Nic-Ma Nic-Ma approved these changes

@wyli wyli Awaiting requested review from wyli

@dongyang0122 dongyang0122 Awaiting requested review from dongyang0122

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mingxin-zheng @Nic-Ma @wyli