Skip to content

Commit de384ec

Browse files
mauvilsatsenst
authored andcommitted
LinghtningCLI now will not allow setting a class instance as a default (Lightning-AI#18822)
1 parent 6f6f45b commit de384ec

File tree

4 files changed

+11
-8
lines changed

4 files changed

+11
-8
lines changed

requirements/pytorch/extra.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
matplotlib>3.1, <3.9.0
66
omegaconf >=2.0.5, <2.4.0
77
hydra-core >=1.0.5, <1.4.0
8-
jsonargparse[signatures] >=4.18.0, <4.26.0
8+
jsonargparse[signatures] >=4.26.1, <4.27.0
99
rich >=12.3.0, <13.6.0
1010
tensorboardX >=2.2, <2.7.0 # min version is set by torch.onnx missing attribute

src/lightning/pytorch/CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
1414

1515
### Changed
1616

17-
-
17+
- `LightningCLI` no longer allows setting a normal class instance as default. A `lazy_instance` can be used instead ([#18822](https://github.com/Lightning-AI/lightning/pull/18822))
1818

1919

2020
### Deprecated

src/lightning/pytorch/cli.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
from lightning.pytorch.utilities.model_helpers import is_overridden
3131
from lightning.pytorch.utilities.rank_zero import rank_zero_warn
3232

33-
_JSONARGPARSE_SIGNATURES_AVAILABLE = RequirementCache("jsonargparse[signatures]>=4.18.0")
33+
_JSONARGPARSE_SIGNATURES_AVAILABLE = RequirementCache("jsonargparse[signatures]>=4.26.1")
3434

3535
if _JSONARGPARSE_SIGNATURES_AVAILABLE:
3636
import docstring_parser

tests/tests_pytorch/test_cli.py

+8-5
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@
6363
else:
6464
from argparse import Namespace
6565

66+
def lazy_instance(*args, **kwargs):
67+
return None
68+
6669

6770
@contextmanager
6871
def mock_subclasses(baseclass, *subclasses):
@@ -176,7 +179,9 @@ def on_fit_start(self):
176179
self.trainer.ran_asserts = True
177180

178181
with mock.patch("sys.argv", ["any.py", "fit", f"--trainer.callbacks={json.dumps(callbacks)}"]):
179-
cli = LightningCLI(TestModel, trainer_defaults={"fast_dev_run": True, "logger": CSVLogger(".")})
182+
cli = LightningCLI(
183+
TestModel, trainer_defaults={"fast_dev_run": True, "logger": lazy_instance(CSVLogger, save_dir=".")}
184+
)
180185

181186
assert cli.trainer.ran_asserts
182187

@@ -592,7 +597,7 @@ def on_fit_start(self):
592597

593598
# mps not yet supported by distributed
594599
@RunIf(skip_windows=True, mps=False)
595-
@pytest.mark.parametrize("logger", [False, TensorBoardLogger(".")])
600+
@pytest.mark.parametrize("logger", [False, lazy_instance(TensorBoardLogger, save_dir=".")])
596601
@pytest.mark.parametrize("strategy", ["ddp_spawn", "ddp"])
597602
def test_cli_distributed_save_config_callback(cleandir, logger, strategy):
598603
from torch.multiprocessing import ProcessRaisedException
@@ -1478,9 +1483,7 @@ def test_tensorboard_logger_init_args():
14781483
"TensorBoardLogger",
14791484
{
14801485
"save_dir": "tb", # Resolve from TensorBoardLogger.__init__
1481-
},
1482-
{
1483-
"comment": "tb", # Unsupported resolving from local imports
1486+
"comment": "tb", # Resolve from FabricTensorBoardLogger.experiment SummaryWriter local import
14841487
},
14851488
)
14861489

0 commit comments

Comments
 (0)