Deprecate `LightningLoggerBase.close` #9422

jjenniferdai · 2021-09-09T21:25:18Z

What does this PR do?

Fixes #9421
deprecate LightningLoggerBase.close

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

Was this discussed/approved via a GitHub issue? (not for typos and docs)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together?
Did you make sure to update the documentation with your changes? (if necessary)
Did you write any new necessary tests? (not for typos and docs)
Did you verify new and existing tests pass locally with your changes?
Did you list all the breaking changes introduced by this pull request?
Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

Is this pull request ready for review? (if not, please submit in draft mode)
Check that all items from Before submitting are resolved
Make sure the title is self-explanatory and the description concisely explains the PR
Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

pytorch_lightning/loggers/base.py

for more information, see https://pre-commit.ci

…iferdai/pytorch-lightning into deprecate-loggerbase-close

edward-io · 2021-09-09T22:53:34Z

Looks great, you can convert this into a non-draft PR when you're ready.

CHANGELOG.md

jjenniferdai · 2021-09-10T04:04:51Z

trying to figure out the make-docs failure. (found similar error in #9366 but the suggestion doesn't seem to apply here)

pytorch_lightning/loggers/base.py

for more information, see https://pre-commit.ci

pytorch_lightning/loggers/base.py

for more information, see https://pre-commit.ci

tests/loggers/test_base.py

codecov · 2021-09-10T17:25:58Z

Codecov Report

Merging #9422 (7ca038b) into master (089ae9b) will decrease coverage by 3%.
The diff coverage is 95%.

❗ Current head 7ca038b differs from pull request most recent head df63517. Consider uploading reports for the commit df63517 to get more accurate results

@@           Coverage Diff           @@
##           master   #9422    +/-   ##
=======================================
- Coverage      92%     89%    -3%     
=======================================
  Files         179     180     +1     
  Lines       14905   15046   +141     
=======================================
- Hits        13751   13358   -393     
- Misses       1154    1688   +534

pytorch_lightning/loggers/base.py

for more information, see https://pre-commit.ci

pytorch_lightning/loggers/base.py

for more information, see https://pre-commit.ci

pytorch_lightning/loggers/base.py

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

tchaton

LGTM !

for more information, see https://pre-commit.ci

* deprecate loggerbase.close * deprecate warning * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add to changelog * fix import * fix import alphabetize * spacing? * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * copy-paste avoid pre-commit.ci? * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * literally match the other comment * unindent * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * suggest finalize instead of save * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update tests/loggers/test_base.py * format but to be formatted * Update pytorch_lightning/loggers/base.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update pytorch_lightning/loggers/base.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update pytorch_lightning/loggers/base.py Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: ananthsub <ananth.subramaniam@gmail.com> Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: thomas chaton <thomas@grid.ai> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

deprecate loggerbase.close

1467afe

ananthsub added deprecation Includes a deprecation logger Related to the Loggers labels Sep 9, 2021

ananthsub reviewed Sep 9, 2021

View reviewed changes

pytorch_lightning/loggers/base.py Show resolved Hide resolved

jjenniferdai and others added 2 commits September 9, 2021 15:27

deprecate warning

4ce8cca

[pre-commit.ci] auto fixes from pre-commit.com hooks

535b990

for more information, see https://pre-commit.ci

jjenniferdai changed the title ~~deprecate LightningLoggerBase.close~~ [wip] deprecate LightningLoggerBase.close Sep 9, 2021

jjenniferdai added 4 commits September 9, 2021 15:41

add to changelog

637dbcd

Merge branch 'deprecate-loggerbase-close' of https://github.com/jjenn…

06d7646

…iferdai/pytorch-lightning into deprecate-loggerbase-close

fix import

dc1da14

fix import alphabetize

0e3474a

ananthsub reviewed Sep 10, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

ananthsub reviewed Sep 10, 2021

View reviewed changes

pytorch_lightning/loggers/base.py Outdated Show resolved Hide resolved

jjenniferdai and others added 7 commits September 9, 2021 21:17

spacing?

76f1c26

[pre-commit.ci] auto fixes from pre-commit.com hooks

c8f4618

for more information, see https://pre-commit.ci

copy-paste avoid pre-commit.ci?

c707a9f

[pre-commit.ci] auto fixes from pre-commit.com hooks

d6d16ff

for more information, see https://pre-commit.ci

literally match the other comment

5c77e69

unindent

dcb62e6

[pre-commit.ci] auto fixes from pre-commit.com hooks

f6889e0

for more information, see https://pre-commit.ci

awaelchli reviewed Sep 10, 2021

View reviewed changes

pytorch_lightning/loggers/base.py Outdated Show resolved Hide resolved

jjenniferdai and others added 2 commits September 10, 2021 09:00

suggest finalize instead of save

843e72c

[pre-commit.ci] auto fixes from pre-commit.com hooks

6d12d69

for more information, see https://pre-commit.ci

ananthsub reviewed Sep 10, 2021

View reviewed changes

tests/loggers/test_base.py Outdated Show resolved Hide resolved

Update tests/loggers/test_base.py

df63517

format but to be formatted

237bad8

awaelchli reviewed Sep 11, 2021

View reviewed changes

pytorch_lightning/loggers/base.py Outdated Show resolved Hide resolved

[pre-commit.ci] auto fixes from pre-commit.com hooks

9b4ada8

for more information, see https://pre-commit.ci

awaelchli reviewed Sep 12, 2021

View reviewed changes

pytorch_lightning/loggers/base.py Outdated Show resolved Hide resolved

awaelchli and others added 2 commits September 12, 2021 03:36

Update pytorch_lightning/loggers/base.py

5bf9f62

[pre-commit.ci] auto fixes from pre-commit.com hooks

dd0f947

for more information, see https://pre-commit.ci

awaelchli added this to the v1.5 milestone Sep 12, 2021

Borda reviewed Sep 14, 2021

View reviewed changes

pytorch_lightning/loggers/base.py Outdated Show resolved Hide resolved

Update pytorch_lightning/loggers/base.py

651196c

Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>

tchaton approved these changes Sep 14, 2021

View reviewed changes

pre-commit-ci bot and others added 2 commits September 14, 2021 10:04

[pre-commit.ci] auto fixes from pre-commit.com hooks

d0f7ae7

for more information, see https://pre-commit.ci

Merge branch 'master' into deprecate-loggerbase-close

0b1a55c

ananthsub marked this pull request as ready for review September 14, 2021 17:52

ananthsub requested review from carmocca, justusschock, kaushikb11, SeanNaren and williamFalcon as code owners September 14, 2021 17:52

carmocca changed the title ~~[wip] deprecate LightningLoggerBase.close~~ Deprecate LightningLoggerBase.close Sep 15, 2021

carmocca approved these changes Sep 15, 2021

View reviewed changes

mergify bot added the ready PRs ready to be merged label Sep 15, 2021

ananthsub approved these changes Sep 17, 2021

View reviewed changes

extra line

1c6aea8

jjenniferdai mentioned this pull request Sep 20, 2021

seemingly correct format reformatted? PyCQA/docformatter#78

Closed

ananthsub enabled auto-merge (squash) September 20, 2021 21:43

ananthsub merged commit cc77367 into Lightning-AI:master Sep 20, 2021

jjenniferdai deleted the deprecate-loggerbase-close branch September 21, 2021 17:51

akihironitta mentioned this pull request Oct 27, 2022

Update pre-commit #15344

Closed

7 tasks

Borda mentioned this pull request Mar 10, 2023

Remove the deprecated logger.close #13149

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate `LightningLoggerBase.close` #9422

Deprecate `LightningLoggerBase.close` #9422

jjenniferdai commented Sep 9, 2021 •

edited by carmocca

Loading

edward-io commented Sep 9, 2021

jjenniferdai commented Sep 10, 2021

codecov bot commented Sep 10, 2021

tchaton left a comment

Deprecate LightningLoggerBase.close #9422

Deprecate LightningLoggerBase.close #9422

Conversation

jjenniferdai commented Sep 9, 2021 • edited by carmocca Loading

What does this PR do?

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

PR review

Did you have fun?

edward-io commented Sep 9, 2021

jjenniferdai commented Sep 10, 2021

codecov bot commented Sep 10, 2021

Codecov Report

tchaton left a comment

Choose a reason for hiding this comment

Deprecate `LightningLoggerBase.close` #9422

Deprecate `LightningLoggerBase.close` #9422

jjenniferdai commented Sep 9, 2021 •

edited by carmocca

Loading