Skip to content

Commit

Permalink
Deprecate LightningLoggerBase.close (#9422)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
6 people authored Sep 20, 2021
1 parent 2e17b47 commit cc77367
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Deprecated passing `flush_logs_every_n_steps` as a Trainer argument, instead pass it to the logger init if supported ([#9366](https://github.com/PyTorchLightning/pytorch-lightning/pull/9366))


- Deprecated `LightningLoggerBase.close`, `LoggerCollection.close` in favor of `LightningLoggerBase.finalize`, `LoggerCollection.finalize` ([#9422](https://github.com/PyTorchLightning/pytorch-lightning/pull/9422))



### Removed

Expand Down
23 changes: 22 additions & 1 deletion pytorch_lightning/loggers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import pytorch_lightning as pl
from pytorch_lightning.callbacks.model_checkpoint import ModelCheckpoint
from pytorch_lightning.utilities import rank_zero_only
from pytorch_lightning.utilities.warnings import rank_zero_deprecation


def rank_zero_experiment(fn: Callable) -> Callable:
Expand Down Expand Up @@ -310,7 +311,18 @@ def finalize(self, status: str) -> None:
self.save()

def close(self) -> None:
"""Do any cleanup that is necessary to close an experiment."""
"""Do any cleanup that is necessary to close an experiment.
See deprecation warning below.
.. deprecated:: v1.5
This method is deprecated in v1.5 and will be removed in v1.7.
Please use `LightningLoggerBase.finalize` instead.
"""
rank_zero_deprecation(
"`LightningLoggerBase.close` method is deprecated in v1.5 and will be removed in v1.7."
" Please use `LightningLoggerBase.finalize` instead."
)
self.save()

@property
Expand Down Expand Up @@ -392,6 +404,15 @@ def finalize(self, status: str) -> None:
logger.finalize(status)

def close(self) -> None:
"""
.. deprecated:: v1.5
This method is deprecated in v1.5 and will be removed in v1.7.
Please use `LoggerCollection.finalize` instead.
"""
rank_zero_deprecation(
"`LoggerCollection.close` method is deprecated in v1.5 and will be removed in v1.7."
" Please use `LoggerCollection.finalize` instead."
)
for logger in self._logger_iterable:
logger.close()

Expand Down
16 changes: 15 additions & 1 deletion tests/deprecated_api/test_remove_1-7.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
import torch

from pytorch_lightning import Callback, LightningDataModule, Trainer
from pytorch_lightning.loggers import TestTubeLogger
from pytorch_lightning.loggers import LoggerCollection, TestTubeLogger
from tests.deprecated_api import _soft_unimport_module
from tests.helpers import BoringModel
from tests.helpers.datamodules import MNISTDataModule
from tests.helpers.runif import RunIf
from tests.loggers.test_base import CustomLogger


def test_v1_7_0_deprecated_lightning_module_summarize(tmpdir):
Expand Down Expand Up @@ -224,3 +225,16 @@ def test_v1_7_0_deprecate_add_get_queue(tmpdir):

with pytest.deprecated_call(match=r"`LightningModule.get_from_queue` method was deprecated in v1.5"):
trainer.fit(model)


def test_v1_7_0_lightning_logger_base_close(tmpdir):
logger = CustomLogger()
with pytest.deprecated_call(
match="`LightningLoggerBase.close` method is deprecated in v1.5 and will be removed in v1.7."
):
logger.close()
with pytest.deprecated_call(
match="`LoggerCollection.close` method is deprecated in v1.5 and will be removed in v1.7."
):
logger = LoggerCollection([logger])
logger.close()
8 changes: 4 additions & 4 deletions tests/loggers/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ def test_logger_collection():
mock1.agg_and_log_metrics.assert_called_once_with({"test": 2.0}, 4)
mock2.agg_and_log_metrics.assert_called_once_with({"test": 2.0}, 4)

logger.close()
mock1.close.assert_called_once()
mock2.close.assert_called_once()
logger.finalize("success")
mock1.finalize.assert_called_once()
mock2.finalize.assert_called_once()


class CustomLogger(LightningLoggerBase):
Expand Down Expand Up @@ -211,7 +211,7 @@ def log_metrics(self, metrics, step):
logger.agg_and_log_metrics({"loss": loss}, step=int(i / 5))

assert logger.history == {0: {"loss": 0.5623850983416314}}
logger.close()
logger.save()
assert logger.history == {0: {"loss": 0.5623850983416314}, 1: {"loss": 0.4778883735637184}}


Expand Down

0 comments on commit cc77367

Please sign in to comment.