Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Specify formatter_class=argparse.ArgumentDefaultsHelpFormatter in generic_parsing.py to show default values for arguments in help message #689

Merged
merged 8 commits into from
Mar 15, 2022
Merged

Specify formatter_class=argparse.ArgumentDefaultsHelpFormatter in generic_parsing.py to show default values for arguments in help message #689

merged 8 commits into from
Mar 15, 2022

Conversation

JacopoTeneggi
Copy link
Contributor

@JacopoTeneggi JacopoTeneggi commented Mar 8, 2022 •
edited
Loading

This PR resolves #675 .

Now, default argument values are shown in the help message, for example:

-h, --help                           show this help message and exit
--subscription_id SUBSCRIPTION_ID    The ID of your Azure subscription. (default: )
[...]
--docker_shm_size DOCKER_SHM_SIZE    The shared memory in the docker image for the AzureML VMs. (default: 440g)
--hyperdrive [HYPERDRIVE]            If True, use AzureML HyperDrive for run execution. (default: False)

note that some outputs are empty instead of showing a value of None, should this behavior be changed?

PR Checklist:

  • Ensure that your PR is small, and implements one change.
  • Run PyCharm's code cleanup tools on your Python files.
  • Link the correct GitHub issue for tracking.
  • Update the Changelog file: Describe your change in terms of
    Added/Changed/Removed/... in the "Upcoming" section.
  • When merging your PR, replace the default merge message with a description of your PR,
    and if needed a motivation why that change was required.

ant0nsc
ant0nsc previously approved these changes Mar 9, 2022
View reviewed changes
@ant0nsc
Copy link
Contributor

ant0nsc commented Mar 9, 2022 •
edited
Loading

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ant0nsc
Copy link
Contributor

ant0nsc commented Mar 9, 2022

Hey @JacopoTeneggi , Your change looks good, I forgot that we need to start Azure pipelines manually for external contributions, which I did just now.

ant0nsc
ant0nsc previously approved these changes Mar 9, 2022
View reviewed changes
@ant0nsc ant0nsc requested a review from fepegar March 11, 2022 14:53
fepegar
fepegar previously approved these changes Mar 11, 2022
View reviewed changes
Copy link
Contributor

@fepegar fepegar left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @JacopoTeneggi!

@ant0nsc ant0nsc enabled auto-merge (squash) March 11, 2022 15:07
@JacopoTeneggi
Copy link
Contributor Author

Hi @ant0nsc sorry to mention directly: I do not have permissions to merge and I see that the check InnerEye-DeepLearning-PR is waiting for status to be reported. Is there anything I am missing?

@ant0nsc ant0nsc dismissed stale reviews from fepegar and themself via 6922dec March 15, 2022 15:00
@ant0nsc
Copy link
Contributor

ant0nsc commented Mar 15, 2022

@JacopoTeneggi , (no issue about the mentioning, that's helpful really!), I've merged main and will restart the pipelines now.

@ant0nsc
Copy link
Contributor

ant0nsc commented Mar 15, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ant0nsc ant0nsc merged commit 95d8b72 into microsoft:main Mar 15, 2022
@JacopoTeneggi JacopoTeneggi deleted the jteneggi/print_default_values branch March 15, 2022 17:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ant0nsc ant0nsc ant0nsc approved these changes

@fepegar fepegar fepegar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Default values for arguments should be printed out
4 participants
@JacopoTeneggi @ant0nsc @fepegar @javier-alvarez