From cc77367ffcf604074d16f3b57a4dcc4b0ca14702 Mon Sep 17 00:00:00 2001 From: jjenniferdai <89552168+jjenniferdai@users.noreply.github.com> Date: Mon, 20 Sep 2021 15:00:09 -0700 Subject: [PATCH] Deprecate `LightningLoggerBase.close` (#9422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * [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 Co-authored-by: Adrian Wälchli Co-authored-by: thomas chaton Co-authored-by: Jirka Borovec --- CHANGELOG.md | 3 +++ pytorch_lightning/loggers/base.py | 23 ++++++++++++++++++++++- tests/deprecated_api/test_remove_1-7.py | 16 +++++++++++++++- tests/loggers/test_base.py | 8 ++++---- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2462ffbdbb773..437d57a933169 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/pytorch_lightning/loggers/base.py b/pytorch_lightning/loggers/base.py index 5e9ae2a94e335..71e7b9f9025ad 100644 --- a/pytorch_lightning/loggers/base.py +++ b/pytorch_lightning/loggers/base.py @@ -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: @@ -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 @@ -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() diff --git a/tests/deprecated_api/test_remove_1-7.py b/tests/deprecated_api/test_remove_1-7.py index 5d7d7db761233..25ab1ea5fd3cc 100644 --- a/tests/deprecated_api/test_remove_1-7.py +++ b/tests/deprecated_api/test_remove_1-7.py @@ -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): @@ -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() diff --git a/tests/loggers/test_base.py b/tests/loggers/test_base.py index aa97decdede09..0ada58ae0b74f 100644 --- a/tests/loggers/test_base.py +++ b/tests/loggers/test_base.py @@ -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): @@ -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}}