Skip to content

Commit

Permalink
Make unimplemented dataloader hooks raise NotImplementedError (#9161)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tshimanga authored Aug 28, 2021
1 parent 3fd77cb commit c993d0c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- `Trainer.request_dataloader` now takes a `RunningStage` enum instance ([#8858](https://github.com/PyTorchLightning/pytorch-lightning/pull/8858))


- Changed `rank_zero_warn` to `NotImplementedError` in the `{train, val, test, predict}_dataloader` hooks that `Lightning(Data)Module` uses ([#9161](https://github.com/PyTorchLightning/pytorch-lightning/pull/9161))

### Deprecated

- Deprecated `LightningModule.summarize()` in favor of `pytorch_lightning.utilities.model_summary.summarize()`
Expand Down
7 changes: 5 additions & 2 deletions pytorch_lightning/core/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import torch
from torch.optim.optimizer import Optimizer

from pytorch_lightning.utilities import move_data_to_device, rank_zero_warn
from pytorch_lightning.utilities import move_data_to_device
from pytorch_lightning.utilities.types import EVAL_DATALOADERS, STEP_OUTPUT, TRAIN_DATALOADERS


Expand Down Expand Up @@ -540,7 +540,7 @@ def train_dataloader(self):
return {'mnist': mnist_loader, 'cifar': cifar_loader}
"""
rank_zero_warn("`train_dataloader` must be implemented to be used with the Lightning Trainer")
raise NotImplementedError("`train_dataloader` must be implemented to be used with the Lightning Trainer")

def test_dataloader(self) -> EVAL_DATALOADERS:
r"""
Expand Down Expand Up @@ -602,6 +602,7 @@ def test_dataloader(self):
In the case where you return multiple test dataloaders, the :meth:`test_step`
will have an argument ``dataloader_idx`` which matches the order here.
"""
raise NotImplementedError("`test_dataloader` must be implemented to be used with the Lightning Trainer")

def val_dataloader(self) -> EVAL_DATALOADERS:
r"""
Expand Down Expand Up @@ -654,6 +655,7 @@ def val_dataloader(self):
In the case where you return multiple validation dataloaders, the :meth:`validation_step`
will have an argument ``dataloader_idx`` which matches the order here.
"""
raise NotImplementedError("`val_dataloader` must be implemented to be used with the Lightning Trainer")

def predict_dataloader(self) -> EVAL_DATALOADERS:
r"""
Expand All @@ -679,6 +681,7 @@ def predict_dataloader(self) -> EVAL_DATALOADERS:
In the case where you return multiple prediction dataloaders, the :meth:`predict`
will have an argument ``dataloader_idx`` which matches the order here.
"""
raise NotImplementedError("`predict_dataloader` must be implemented to be used with the Lightning Trainer")

def on_train_dataloader(self) -> None:
"""Called before requesting the train dataloader."""
Expand Down
15 changes: 10 additions & 5 deletions tests/core/test_datamodules.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,10 @@ def test_dm_init_from_datasets_dataloaders(iterable):
with mock.patch("pytorch_lightning.core.datamodule.DataLoader") as dl_mock:
dm.train_dataloader()
dl_mock.assert_called_once_with(train_ds, batch_size=4, shuffle=not iterable, num_workers=0, pin_memory=True)
assert dm.val_dataloader() is None
assert dm.test_dataloader() is None
with pytest.raises(NotImplementedError):
_ = dm.val_dataloader()
with pytest.raises(NotImplementedError):
_ = dm.test_dataloader()

train_ds_sequence = [ds(), ds()]
dm = LightningDataModule.from_datasets(train_ds_sequence, batch_size=4, num_workers=0)
Expand All @@ -493,8 +495,10 @@ def test_dm_init_from_datasets_dataloaders(iterable):
call(train_ds_sequence[1], batch_size=4, shuffle=not iterable, num_workers=0, pin_memory=True),
]
)
assert dm.val_dataloader() is None
assert dm.test_dataloader() is None
with pytest.raises(NotImplementedError):
_ = dm.val_dataloader()
with pytest.raises(NotImplementedError):
_ = dm.test_dataloader()

valid_ds = ds()
test_ds = ds()
Expand All @@ -504,7 +508,8 @@ def test_dm_init_from_datasets_dataloaders(iterable):
dl_mock.assert_called_with(valid_ds, batch_size=2, shuffle=False, num_workers=0, pin_memory=True)
dm.test_dataloader()
dl_mock.assert_called_with(test_ds, batch_size=2, shuffle=False, num_workers=0, pin_memory=True)
assert dm.train_dataloader() is None
with pytest.raises(NotImplementedError):
_ = dm.train_dataloader()

valid_dss = [ds(), ds()]
test_dss = [ds(), ds()]
Expand Down

0 comments on commit c993d0c

Please sign in to comment.