Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auto_insert_metric_name to ModelCheckpoint docstring. #8310

Merged
merged 5 commits into from Jul 7, 2021
Merged

Add auto_insert_metric_name to ModelCheckpoint docstring. #8310

merged 5 commits into from Jul 7, 2021

Conversation

jiwidi
Copy link
Contributor

@jiwidi jiwidi commented Jul 6, 2021

What does this PR do?

Hi!

Boring PR fixind docs, adds a missing parameter in the ModelCheckpoint docstring: auto_insert_metric_name. This metric was added on #6277 but the docstring was missed. I found myself in need of this parameter and didn't see anything in the docstring so I went ask in Slack and @carmocca pointed me that this parameter existed, just that it wasn't on the docs! conversation

The parameter is used in code examples but lacks of any explanation https://pytorch-lightning.readthedocs.io/en/latest/extensions/generated/pytorch_lightning.callbacks.ModelCheckpoint.html#pytorch_lightning.callbacks.ModelCheckpoint. This will make it easier to understand

Did you have fun?

Always 馃檭

Added auto_insert_metric_name in modelcheckpoint docstring
@carmocca carmocca added this to the v1.4 milestone Jul 6, 2021
@carmocca carmocca added the docs Documentation related label Jul 6, 2021
@codecov
Copy link

codecov bot commented Jul 6, 2021

Codecov Report

Merging #8310 (fda9453) into master (4184d7e) will decrease coverage by 5%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #8310    +/-   ##
=======================================
- Coverage      93%     88%    -5%     
=======================================
  Files         213     213            
  Lines       13798   13798            
=======================================
- Hits        12799   12121   -678     
- Misses        999    1677   +678     

Copy link
Contributor

@carmocca carmocca left a comment

Choose a reason for hiding this comment

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

Thanks!

pytorch_lightning/callbacks/model_checkpoint.py Outdated Show resolved Hide resolved
pytorch_lightning/callbacks/model_checkpoint.py Outdated Show resolved Hide resolved
pytorch_lightning/callbacks/model_checkpoint.py Outdated Show resolved Hide resolved
pytorch_lightning/callbacks/model_checkpoint.py Outdated Show resolved Hide resolved
jiwidi and others added 4 commits July 6, 2021 18:39
Co-authored-by: Carlos Mochol铆 <carlossmocholi@gmail.com>
Co-authored-by: Carlos Mochol铆 <carlossmocholi@gmail.com>
Co-authored-by: Carlos Mochol铆 <carlossmocholi@gmail.com>
Co-authored-by: Carlos Mochol铆 <carlossmocholi@gmail.com>
@carmocca carmocca added the ready PRs ready to be merged label Jul 7, 2021
@awaelchli awaelchli enabled auto-merge (squash) July 7, 2021 22:54
@awaelchli awaelchli merged commit 9bbca40 into Lightning-AI:master Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants