From ad7da4f049aa246fef5855155163d3d796d613c8 Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Mon, 28 Feb 2022 12:56:43 -0800 Subject: [PATCH 01/10] deprecate LoggerCollection --- pytorch_lightning/trainer/trainer.py | 9 +++++---- tests/deprecated_api/test_remove_1-8.py | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index 46e9aeb368c90..cac1603300f5d 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2599,10 +2599,11 @@ def logger(self) -> Optional[LightningLoggerBase]: if len(self.loggers) == 1: return self.loggers[0] else: - rank_zero_warn( - "Using trainer.logger when Trainer is configured to use multiple loggers." - " This behavior will change in v1.8 when LoggerCollection is removed, and" - " trainer.logger will return the first logger in trainer.loggers" + rank_zero_deprecation( + "Using `trainer.logger` when Trainer is configured to use multiple loggers." + " `LoggerCollection` is deprecated in v1.6 and will be removed in v1.8." + " This behavior will change in v1.8 such that `trainer.logger` will return the first" + " logger in `trainer.loggers`." ) return LoggerCollection(self.loggers) diff --git a/tests/deprecated_api/test_remove_1-8.py b/tests/deprecated_api/test_remove_1-8.py index 09e82ddbe3ae3..d265d58d7a6af 100644 --- a/tests/deprecated_api/test_remove_1-8.py +++ b/tests/deprecated_api/test_remove_1-8.py @@ -652,3 +652,24 @@ def _get_python_cprofile_total_duration(profile): recorded_total_duration = _get_python_cprofile_total_duration(advanced_profiler.profiled_actions[action]) expected_total_duration = np.sum(expected) np.testing.assert_allclose(recorded_total_duration, expected_total_duration, rtol=0.2) + + +def test_v1_8_0_logger_collection(tmpdir): + logger1 = CSVLogger(tmpdir) + logger2 = CSVLogger(tmpdir) + + trainer1 = Trainer(logger=logger1) + trainer2 = Trainer(logger=[logger1, logger2]) + + # Should have no deprecation warning + trainer1.logger + trainer1.loggers + trainer2.loggers + + with pytest.deprecated_call( + match="Using `trainer.logger` when Trainer is configured to use multiple loggers." + " `LoggerCollection` is deprecated in v1.6 and will be removed in v1.8." + " This behavior will change in v1.8 such that `trainer.logger` will return the first" + " logger in `trainer.loggers`." + ): + trainer1.logger From b519a1615d790d8fc7fc939cab0baf6d2631a31b Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Mon, 28 Feb 2022 13:05:25 -0800 Subject: [PATCH 02/10] update changelog --- CHANGELOG.md | 3 +++ pytorch_lightning/trainer/trainer.py | 2 +- tests/deprecated_api/test_remove_1-8.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9606b7d653aed..adb68840f8bae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -445,6 +445,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Deprecated `BaseProfiler.profile_iterable` ([#12102](https://github.com/PyTorchLightning/pytorch-lightning/pull/12102)) +- Deprecated `LoggerCollection` in favor of `trainer.loggers` ([#12147](https://github.com/PyTorchLightning/pytorch-lightning/pull/12147)) + + ### Removed - Removed deprecated parameter `method` in `pytorch_lightning.utilities.model_helpers.is_overridden` ([#10507](https://github.com/PyTorchLightning/pytorch-lightning/pull/10507)) diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index cac1603300f5d..a6ab189f08a0f 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2601,7 +2601,7 @@ def logger(self) -> Optional[LightningLoggerBase]: else: rank_zero_deprecation( "Using `trainer.logger` when Trainer is configured to use multiple loggers." - " `LoggerCollection` is deprecated in v1.6 and will be removed in v1.8." + " `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." " This behavior will change in v1.8 such that `trainer.logger` will return the first" " logger in `trainer.loggers`." ) diff --git a/tests/deprecated_api/test_remove_1-8.py b/tests/deprecated_api/test_remove_1-8.py index d265d58d7a6af..1f9cebeb6d04a 100644 --- a/tests/deprecated_api/test_remove_1-8.py +++ b/tests/deprecated_api/test_remove_1-8.py @@ -668,7 +668,7 @@ def test_v1_8_0_logger_collection(tmpdir): with pytest.deprecated_call( match="Using `trainer.logger` when Trainer is configured to use multiple loggers." - " `LoggerCollection` is deprecated in v1.6 and will be removed in v1.8." + " `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." " This behavior will change in v1.8 such that `trainer.logger` will return the first" " logger in `trainer.loggers`." ): From a6bcbf77fbfacf2bc3a40e10992e6dff75594cc0 Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Mon, 28 Feb 2022 13:36:52 -0800 Subject: [PATCH 03/10] fix unit tests --- tests/deprecated_api/test_remove_1-8.py | 2 +- tests/profiler/test_profiler.py | 14 +++++++------- tests/trainer/properties/test_log_dir.py | 7 +++---- tests/trainer/properties/test_loggers.py | 12 ++++++++++-- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/tests/deprecated_api/test_remove_1-8.py b/tests/deprecated_api/test_remove_1-8.py index 1f9cebeb6d04a..4032050edd719 100644 --- a/tests/deprecated_api/test_remove_1-8.py +++ b/tests/deprecated_api/test_remove_1-8.py @@ -672,4 +672,4 @@ def test_v1_8_0_logger_collection(tmpdir): " This behavior will change in v1.8 such that `trainer.logger` will return the first" " logger in `trainer.loggers`." ): - trainer1.logger + trainer2.logger diff --git a/tests/profiler/test_profiler.py b/tests/profiler/test_profiler.py index 161c9e6e35670..24f6c70e72b88 100644 --- a/tests/profiler/test_profiler.py +++ b/tests/profiler/test_profiler.py @@ -24,7 +24,7 @@ from pytorch_lightning import Callback, Trainer from pytorch_lightning.callbacks import EarlyStopping, StochasticWeightAveraging -from pytorch_lightning.loggers import CSVLogger, LoggerCollection, TensorBoardLogger +from pytorch_lightning.loggers import CSVLogger, TensorBoardLogger from pytorch_lightning.profiler import AdvancedProfiler, PassThroughProfiler, PyTorchProfiler, SimpleProfiler from pytorch_lightning.profiler.pytorch import RegisterRecordFunction, warning_cache from pytorch_lightning.utilities.exceptions import MisconfigurationException @@ -450,9 +450,9 @@ def test_pytorch_profiler_nested(tmpdir): assert events_name == expected, (events_name, torch.__version__, platform.system()) -def test_pytorch_profiler_logger_collection(tmpdir): - """Tests whether the PyTorch profiler is able to write its trace locally when the Trainer's logger is an - instance of LoggerCollection. +def test_pytorch_profiler_multiple_loggers(tmpdir): + """Tests whether the PyTorch profiler is able to write its trace locally when the Trainer is configured with + multiple loggers. See issue #8157. """ @@ -466,9 +466,9 @@ def look_for_trace(trace_dir): model = BoringModel() # Wrap the logger in a list so it becomes a LoggerCollection - logger = [TensorBoardLogger(save_dir=tmpdir), CSVLogger(tmpdir)] - trainer = Trainer(default_root_dir=tmpdir, profiler="pytorch", logger=logger, limit_train_batches=5, max_epochs=1) - assert isinstance(trainer.logger, LoggerCollection) + loggers = [TensorBoardLogger(save_dir=tmpdir), CSVLogger(tmpdir)] + trainer = Trainer(default_root_dir=tmpdir, profiler="pytorch", logger=loggers, limit_train_batches=5, max_epochs=1) + assert len(trainer.loggers) == 2 trainer.fit(model) assert look_for_trace(tmpdir) diff --git a/tests/trainer/properties/test_log_dir.py b/tests/trainer/properties/test_log_dir.py index 6777ec8183737..db2f862f82e7b 100644 --- a/tests/trainer/properties/test_log_dir.py +++ b/tests/trainer/properties/test_log_dir.py @@ -15,7 +15,7 @@ from pytorch_lightning import Trainer from pytorch_lightning.callbacks import ModelCheckpoint -from pytorch_lightning.loggers import CSVLogger, LoggerCollection, TensorBoardLogger +from pytorch_lightning.loggers import CSVLogger, TensorBoardLogger from tests.helpers.boring_model import BoringModel @@ -109,8 +109,8 @@ def test_logdir_custom_logger(tmpdir): assert trainer.log_dir == expected -def test_logdir_logger_collection(tmpdir): - """Tests that the logdir equals the default_root_dir when the logger is a LoggerCollection.""" +def test_logdir_multiple_loggers(tmpdir): + """Tests that the logdir equals the default_root_dir when trainer has multiple loggers.""" default_root_dir = tmpdir / "default_root_dir" save_dir = tmpdir / "save_dir" model = TestModel(default_root_dir) @@ -119,7 +119,6 @@ def test_logdir_logger_collection(tmpdir): max_steps=2, logger=[TensorBoardLogger(save_dir=save_dir, name="custom_logs"), CSVLogger(tmpdir)], ) - assert isinstance(trainer.logger, LoggerCollection) assert trainer.log_dir == default_root_dir trainer.fit(model) diff --git a/tests/trainer/properties/test_loggers.py b/tests/trainer/properties/test_loggers.py index d3db78986f361..f1075f2dc1c90 100644 --- a/tests/trainer/properties/test_loggers.py +++ b/tests/trainer/properties/test_loggers.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest + from pytorch_lightning import Trainer from pytorch_lightning.loggers import LoggerCollection, TensorBoardLogger from tests.loggers.test_base import CustomLogger @@ -52,6 +54,10 @@ def test_trainer_loggers_setters(): logger2 = CustomLogger() logger_collection = LoggerCollection([logger1, logger2]) logger_collection_2 = LoggerCollection([logger2]) + logger_collection_dep = "Using `trainer.logger` when Trainer is configured to use multiple loggers." + " `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." + " This behavior will change in v1.8 such that `trainer.logger` will return the first" + " logger in `trainer.loggers`." trainer = Trainer() assert type(trainer.logger) == TensorBoardLogger @@ -63,7 +69,8 @@ def test_trainer_loggers_setters(): assert trainer.loggers == [logger1] trainer.logger = logger_collection - assert trainer.logger._logger_iterable == logger_collection._logger_iterable + with pytest.deprecated_call(match=logger_collection_dep): + assert trainer.logger._logger_iterable == logger_collection._logger_iterable assert trainer.loggers == [logger1, logger2] # LoggerCollection of size 1 should result in trainer.logger becoming the contained logger. @@ -78,7 +85,8 @@ def test_trainer_loggers_setters(): # Test setters for trainer.loggers trainer.loggers = [logger1, logger2] assert trainer.loggers == [logger1, logger2] - assert trainer.logger._logger_iterable == logger_collection._logger_iterable + with pytest.deprecated_call(match=logger_collection_dep): + assert trainer.logger._logger_iterable == logger_collection._logger_iterable trainer.loggers = [logger1] assert trainer.loggers == [logger1] From 2efd7776ad9c32c6635f031d930afa92743842df Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Mon, 28 Feb 2022 14:58:35 -0800 Subject: [PATCH 04/10] Split deprecation message --- pytorch_lightning/loggers/base.py | 6 ++++++ pytorch_lightning/trainer/trainer.py | 1 - tests/deprecated_api/test_remove_1-8.py | 14 +++++++------- tests/trainer/properties/test_loggers.py | 11 +++++------ 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/pytorch_lightning/loggers/base.py b/pytorch_lightning/loggers/base.py index f0a8ba13dbdcd..dae253e393433 100644 --- a/pytorch_lightning/loggers/base.py +++ b/pytorch_lightning/loggers/base.py @@ -221,6 +221,9 @@ def version(self) -> Union[int, str]: class LoggerCollection(LightningLoggerBase): """The :class:`LoggerCollection` class is used to iterate all logging actions over the given `logger_iterable`. + .. deprecated:: v1.6 + `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8. + Args: logger_iterable: An iterable collection of loggers """ @@ -228,6 +231,9 @@ class LoggerCollection(LightningLoggerBase): def __init__(self, logger_iterable: Iterable[LightningLoggerBase]): super().__init__() self._logger_iterable = logger_iterable + rank_zero_deprecation( + "`LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." + ) def __getitem__(self, index: int) -> LightningLoggerBase: return list(self._logger_iterable)[index] diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index a6ab189f08a0f..355c75b87d562 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2601,7 +2601,6 @@ def logger(self) -> Optional[LightningLoggerBase]: else: rank_zero_deprecation( "Using `trainer.logger` when Trainer is configured to use multiple loggers." - " `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." " This behavior will change in v1.8 such that `trainer.logger` will return the first" " logger in `trainer.loggers`." ) diff --git a/tests/deprecated_api/test_remove_1-8.py b/tests/deprecated_api/test_remove_1-8.py index 4032050edd719..68c03463036a0 100644 --- a/tests/deprecated_api/test_remove_1-8.py +++ b/tests/deprecated_api/test_remove_1-8.py @@ -21,7 +21,7 @@ from torch import optim from pytorch_lightning import Callback, Trainer -from pytorch_lightning.loggers import CSVLogger, LightningLoggerBase +from pytorch_lightning.loggers import CSVLogger, LightningLoggerBase, LoggerCollection from pytorch_lightning.plugins.training_type.ddp import DDPPlugin from pytorch_lightning.plugins.training_type.ddp2 import DDP2Plugin from pytorch_lightning.plugins.training_type.ddp_spawn import DDPSpawnPlugin @@ -666,10 +666,10 @@ def test_v1_8_0_logger_collection(tmpdir): trainer1.loggers trainer2.loggers - with pytest.deprecated_call( - match="Using `trainer.logger` when Trainer is configured to use multiple loggers." - " `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." - " This behavior will change in v1.8 such that `trainer.logger` will return the first" - " logger in `trainer.loggers`." - ): + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + trainer2.logger + + with pytest.deprecated_call(match="Using `trainer.logger` when Trainer is configured to use multiple loggers."): trainer2.logger + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + LoggerCollection([logger1, logger2]) diff --git a/tests/trainer/properties/test_loggers.py b/tests/trainer/properties/test_loggers.py index f1075f2dc1c90..b24b3646832a5 100644 --- a/tests/trainer/properties/test_loggers.py +++ b/tests/trainer/properties/test_loggers.py @@ -52,12 +52,11 @@ def test_trainer_loggers_setters(): """Test the behavior of setters for trainer.logger and trainer.loggers.""" logger1 = CustomLogger() logger2 = CustomLogger() - logger_collection = LoggerCollection([logger1, logger2]) - logger_collection_2 = LoggerCollection([logger2]) - logger_collection_dep = "Using `trainer.logger` when Trainer is configured to use multiple loggers." - " `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." - " This behavior will change in v1.8 such that `trainer.logger` will return the first" - " logger in `trainer.loggers`." + logger_collection_dep = "`LoggerCollection` is deprecated in v1.6" + with pytest.deprecated_call(match=logger_collection_dep): + logger_collection = LoggerCollection([logger1, logger2]) + with pytest.deprecated_call(match=logger_collection_dep): + logger_collection_2 = LoggerCollection([logger2]) trainer = Trainer() assert type(trainer.logger) == TensorBoardLogger From 6b26948b25915b59f3c0ff9f500db82b237169eb Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Mon, 28 Feb 2022 15:47:11 -0800 Subject: [PATCH 05/10] fix unit tests and change dep to warning --- pytorch_lightning/trainer/trainer.py | 2 +- tests/deprecated_api/test_remove_1-8.py | 2 -- tests/loggers/test_base.py | 15 ++++++++++----- tests/profiler/test_profiler.py | 1 - tests/trainer/properties/test_loggers.py | 9 ++++----- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index 355c75b87d562..ce9d3ae68bd84 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2599,7 +2599,7 @@ def logger(self) -> Optional[LightningLoggerBase]: if len(self.loggers) == 1: return self.loggers[0] else: - rank_zero_deprecation( + rank_zero_warn( "Using `trainer.logger` when Trainer is configured to use multiple loggers." " This behavior will change in v1.8 such that `trainer.logger` will return the first" " logger in `trainer.loggers`." diff --git a/tests/deprecated_api/test_remove_1-8.py b/tests/deprecated_api/test_remove_1-8.py index 68c03463036a0..0e8639a9d111f 100644 --- a/tests/deprecated_api/test_remove_1-8.py +++ b/tests/deprecated_api/test_remove_1-8.py @@ -669,7 +669,5 @@ def test_v1_8_0_logger_collection(tmpdir): with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): trainer2.logger - with pytest.deprecated_call(match="Using `trainer.logger` when Trainer is configured to use multiple loggers."): - trainer2.logger with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): LoggerCollection([logger1, logger2]) diff --git a/tests/loggers/test_base.py b/tests/loggers/test_base.py index cd7eec14eec77..ff93f9933d74b 100644 --- a/tests/loggers/test_base.py +++ b/tests/loggers/test_base.py @@ -34,7 +34,8 @@ def test_logger_collection(): mock1 = MagicMock() mock2 = MagicMock() - logger = LoggerCollection([mock1, mock2]) + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + logger = LoggerCollection([mock1, mock2]) assert logger[0] == mock1 assert logger[1] == mock2 @@ -62,14 +63,16 @@ def test_logger_collection_unique_names(): logger1 = CustomLogger(name=unique_name) logger2 = CustomLogger(name=unique_name) - logger = LoggerCollection([logger1, logger2]) + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + logger = LoggerCollection([logger1, logger2]) assert logger.name == unique_name def test_logger_collection_names_order(): loggers = [CustomLogger(name=n) for n in ("name1", "name2", "name1", "name3")] - logger = LoggerCollection(loggers) + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + logger = LoggerCollection(loggers) assert logger.name == f"{loggers[0].name}_{loggers[1].name}_{loggers[3].name}" @@ -78,14 +81,16 @@ def test_logger_collection_unique_versions(): logger1 = CustomLogger(version=unique_version) logger2 = CustomLogger(version=unique_version) - logger = LoggerCollection([logger1, logger2]) + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + logger = LoggerCollection([logger1, logger2]) assert logger.version == unique_version def test_logger_collection_versions_order(): loggers = [CustomLogger(version=v) for v in ("1", "2", "1", "3")] - logger = LoggerCollection(loggers) + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): + logger = LoggerCollection(loggers) assert logger.version == f"{loggers[0].version}_{loggers[1].version}_{loggers[3].version}" diff --git a/tests/profiler/test_profiler.py b/tests/profiler/test_profiler.py index 24f6c70e72b88..2971ba7bc2c19 100644 --- a/tests/profiler/test_profiler.py +++ b/tests/profiler/test_profiler.py @@ -465,7 +465,6 @@ def look_for_trace(trace_dir): assert not look_for_trace(tmpdir) model = BoringModel() - # Wrap the logger in a list so it becomes a LoggerCollection loggers = [TensorBoardLogger(save_dir=tmpdir), CSVLogger(tmpdir)] trainer = Trainer(default_root_dir=tmpdir, profiler="pytorch", logger=loggers, limit_train_batches=5, max_epochs=1) assert len(trainer.loggers) == 2 diff --git a/tests/trainer/properties/test_loggers.py b/tests/trainer/properties/test_loggers.py index b24b3646832a5..3b3ffeea44310 100644 --- a/tests/trainer/properties/test_loggers.py +++ b/tests/trainer/properties/test_loggers.py @@ -52,10 +52,9 @@ def test_trainer_loggers_setters(): """Test the behavior of setters for trainer.logger and trainer.loggers.""" logger1 = CustomLogger() logger2 = CustomLogger() - logger_collection_dep = "`LoggerCollection` is deprecated in v1.6" - with pytest.deprecated_call(match=logger_collection_dep): + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): logger_collection = LoggerCollection([logger1, logger2]) - with pytest.deprecated_call(match=logger_collection_dep): + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): logger_collection_2 = LoggerCollection([logger2]) trainer = Trainer() @@ -68,7 +67,7 @@ def test_trainer_loggers_setters(): assert trainer.loggers == [logger1] trainer.logger = logger_collection - with pytest.deprecated_call(match=logger_collection_dep): + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): assert trainer.logger._logger_iterable == logger_collection._logger_iterable assert trainer.loggers == [logger1, logger2] @@ -84,7 +83,7 @@ def test_trainer_loggers_setters(): # Test setters for trainer.loggers trainer.loggers = [logger1, logger2] assert trainer.loggers == [logger1, logger2] - with pytest.deprecated_call(match=logger_collection_dep): + with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): assert trainer.logger._logger_iterable == logger_collection._logger_iterable trainer.loggers = [logger1] From 26ddec96f01fd3cfa0e945a84238746037f12c2f Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Tue, 1 Mar 2022 12:12:18 -0800 Subject: [PATCH 06/10] update warning message --- pytorch_lightning/trainer/trainer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index ce9d3ae68bd84..fc26ce436c741 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2600,7 +2600,7 @@ def logger(self) -> Optional[LightningLoggerBase]: return self.loggers[0] else: rank_zero_warn( - "Using `trainer.logger` when Trainer is configured to use multiple loggers." + "You are using `trainer.logger` when the Trainer is configured to use multiple loggers." " This behavior will change in v1.8 such that `trainer.logger` will return the first" " logger in `trainer.loggers`." ) From 80fad1797eec1aff95c2ae9811713fc1d3aa86df Mon Sep 17 00:00:00 2001 From: ananthsub Date: Tue, 1 Mar 2022 19:54:13 -0800 Subject: [PATCH 07/10] Update pytorch_lightning/trainer/trainer.py --- pytorch_lightning/trainer/trainer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index fc26ce436c741..46e9aeb368c90 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2600,9 +2600,9 @@ def logger(self) -> Optional[LightningLoggerBase]: return self.loggers[0] else: rank_zero_warn( - "You are using `trainer.logger` when the Trainer is configured to use multiple loggers." - " This behavior will change in v1.8 such that `trainer.logger` will return the first" - " logger in `trainer.loggers`." + "Using trainer.logger when Trainer is configured to use multiple loggers." + " This behavior will change in v1.8 when LoggerCollection is removed, and" + " trainer.logger will return the first logger in trainer.loggers" ) return LoggerCollection(self.loggers) From 9a3c3f6d848b099ee4229c5c670419b88da0feb4 Mon Sep 17 00:00:00 2001 From: Akash Kwatra Date: Wed, 2 Mar 2022 15:30:53 -0800 Subject: [PATCH 08/10] Update deprecation message --- pytorch_lightning/loggers/base.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pytorch_lightning/loggers/base.py b/pytorch_lightning/loggers/base.py index dae253e393433..dc50d48b23670 100644 --- a/pytorch_lightning/loggers/base.py +++ b/pytorch_lightning/loggers/base.py @@ -222,7 +222,8 @@ class LoggerCollection(LightningLoggerBase): """The :class:`LoggerCollection` class is used to iterate all logging actions over the given `logger_iterable`. .. deprecated:: v1.6 - `LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8. + `LoggerCollection` is deprecated in v1.6 and will be removed in v1.8. + Directly pass a list of loggers to the Trainer and access the list via the `trainer.loggers` attribute. Args: logger_iterable: An iterable collection of loggers @@ -232,7 +233,8 @@ def __init__(self, logger_iterable: Iterable[LightningLoggerBase]): super().__init__() self._logger_iterable = logger_iterable rank_zero_deprecation( - "`LoggerCollection` is deprecated in v1.6 in favor of `trainer.loggers` and will be removed in v1.8." + "`LoggerCollection` is deprecated in v1.6 and will be removed in v1.8. Directly pass a list of loggers" + " to the Trainer and access the list via the `trainer.loggers` attribute." ) def __getitem__(self, index: int) -> LightningLoggerBase: From d7e1f2e8f6dfbacd6c3ade672117746782ae4b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20W=C3=A4lchli?= Date: Fri, 4 Mar 2022 22:02:52 +0100 Subject: [PATCH 09/10] suppress deprecation warning triggered internally --- pytorch_lightning/trainer/trainer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pytorch_lightning/trainer/trainer.py b/pytorch_lightning/trainer/trainer.py index 4c01e58e9e3d5..881b62271ec73 100644 --- a/pytorch_lightning/trainer/trainer.py +++ b/pytorch_lightning/trainer/trainer.py @@ -2623,7 +2623,9 @@ def logger(self) -> Optional[LightningLoggerBase]: " This behavior will change in v1.8 when LoggerCollection is removed, and" " trainer.logger will return the first logger in trainer.loggers" ) - return LoggerCollection(self.loggers) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + return LoggerCollection(self.loggers) @logger.setter def logger(self, logger: Optional[LightningLoggerBase]) -> None: From 13d74094f55c873d7fc36458797dc7abc9aabdf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20W=C3=A4lchli?= Date: Fri, 4 Mar 2022 22:20:48 +0100 Subject: [PATCH 10/10] fix test for unwanted deprecation message --- tests/deprecated_api/test_remove_1-8.py | 4 +--- tests/trainer/properties/test_loggers.py | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/deprecated_api/test_remove_1-8.py b/tests/deprecated_api/test_remove_1-8.py index a9db8e51fbcdc..3c8923a96c3ad 100644 --- a/tests/deprecated_api/test_remove_1-8.py +++ b/tests/deprecated_api/test_remove_1-8.py @@ -673,9 +673,7 @@ def test_v1_8_0_logger_collection(tmpdir): trainer1.logger trainer1.loggers trainer2.loggers - - with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): - trainer2.logger + trainer2.logger with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): LoggerCollection([logger1, logger2]) diff --git a/tests/trainer/properties/test_loggers.py b/tests/trainer/properties/test_loggers.py index 3b3ffeea44310..b48f3f2768175 100644 --- a/tests/trainer/properties/test_loggers.py +++ b/tests/trainer/properties/test_loggers.py @@ -67,8 +67,7 @@ def test_trainer_loggers_setters(): assert trainer.loggers == [logger1] trainer.logger = logger_collection - with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): - assert trainer.logger._logger_iterable == logger_collection._logger_iterable + assert trainer.logger._logger_iterable == logger_collection._logger_iterable assert trainer.loggers == [logger1, logger2] # LoggerCollection of size 1 should result in trainer.logger becoming the contained logger. @@ -83,8 +82,7 @@ def test_trainer_loggers_setters(): # Test setters for trainer.loggers trainer.loggers = [logger1, logger2] assert trainer.loggers == [logger1, logger2] - with pytest.deprecated_call(match="`LoggerCollection` is deprecated in v1.6"): - assert trainer.logger._logger_iterable == logger_collection._logger_iterable + assert trainer.logger._logger_iterable == logger_collection._logger_iterable trainer.loggers = [logger1] assert trainer.loggers == [logger1]