Repository: pytorch/ignite

Issue:
Fix warnings in test
## 🚀 Feature

Currently, we have several warnings raised during the tests from our side but not catched with `pytest.warns`:
- https://travis-ci.org/github/pytorch/ignite/jobs/689725736#L3577
- https://travis-ci.org/github/pytorch/ignite/jobs/689725736#L3589
- https://travis-ci.org/github/pytorch/ignite/jobs/689725736#L3593
- etc

We need to improve the code of tests by introducing
```python
with pytest.warns(...):
    # code that raises a warning
```


Discussion:
InCogNiTo124: I was not able to fix `test_setup_visdom_logging` from `tests/ignite/contrib/engines/test_common.py`

InCogNiTo124: It seems sometimes some warnings are emitted, and other times they aren't, for example `tests/ignite/contrib/handlers/test_lr_finder.py:222`.
InCogNiTo124: `ignite/contrib/handlers/custom_events.py` has a typo in its warning message :D Since it's deprecated, it's not worth fixing it, is it?
vfdev-5: You can fix the typo :)
vfdev-5: @InCogNiTo124 my bad I haven't thought about that warnings can depend on python version. Case of `collection.abc` etc. Maybe we can ignore it as it is not related to our code.
InCogNiTo124: What I'm more worried about is that I can not deal with `ioloop.install()` warning from `test_setup_visdom_logging`. The warning is thrown somewhere deep in the visdom library, and, funnily enough, wherever I'd put `pytest.warns()` it would fail.
vfdev-5: > @vfdev-5 What I'm more worried about is that I can not deal with `ioloop.install()` warning from `test_setup_visdom_logging`. The warning is thrown somewhere deep in the visdom library, and, funnily enough, wherever I'd put `pytest.warns()` it would fail.

I think we can just ignore them as it is not our code is responsible for that warning. Let's remove those `pytest.warns()` that we can not handle and keep only those where we catch our `warning.warn`. What do you think ?
InCogNiTo124: That sounds doable
InCogNiTo124: I have removed the check for version dependant warnings, so I'd say this task is done.
vfdev-5: All warnings from the issue are solved ! 
But seems like I introduced another one with the recent commit. 
https://travis-ci.org/github/pytorch/ignite/jobs/694021111#L2120
Let me fix by myself, I'll commit it here and we can merge. 
Thanks again for working on this PR !
InCogNiTo124: Not a problem @vfdev-5, I'll come back for another one when I find the time :smile:

Patch:
diff --git a/ignite/contrib/engines/common.py b/ignite/contrib/engines/common.py
index 9e6f2fc4..76c25e2b 100644
--- a/ignite/contrib/engines/common.py
+++ b/ignite/contrib/engines/common.py
@@ -97,7 +97,10 @@ def setup_common_training_handlers(
     else:
         if train_sampler is not None and isinstance(train_sampler, DistributedSampler):
             warnings.warn(
-                "Argument train_sampler is a distributed sampler, but no distributed setting detected", UserWarning,
+                "Argument train_sampler is a distributed sampler,"
+                " but either there is no distributed setting or world size is < 2. "
+                "Train sampler argument will be ignored",
+                UserWarning,
             )
         _setup_common_training_handlers(trainer, **kwargs)
 
diff --git a/ignite/contrib/handlers/custom_events.py b/ignite/contrib/handlers/custom_events.py
index 1bdf43fd..1418f0d4 100644
--- a/ignite/contrib/handlers/custom_events.py
+++ b/ignite/contrib/handlers/custom_events.py
@@ -67,7 +67,7 @@ class CustomPeriodicEvent:
             raise ValueError("Argument n_epochs should be positive integer number")
 
         if (n_iterations is None and n_epochs is None) or (n_iterations and n_epochs):
-            raise ValueError("Either n_iterations or n_epochs should defined")
+            raise ValueError("Either n_iterations or n_epochs should be defined")
 
         if n_iterations:
             prefix = "iterations"
diff --git a/ignite/metrics/accumulation.py b/ignite/metrics/accumulation.py
index f03f6f03..c3bbbfc5 100644
--- a/ignite/metrics/accumulation.py
+++ b/ignite/metrics/accumulation.py
@@ -51,7 +51,7 @@ class VariableAccumulation(Metric):
     @reinit__is_reduced
     def reset(self) -> None:
         self.accumulator = torch.tensor(0.0, dtype=torch.float64, device=self._device)
-        self.num_examples = torch.tensor(0.0, dtype=torch.long, device=self._device)
+        self.num_examples = torch.tensor(0, dtype=torch.long, device=self._device)
 
     def _check_output_type(self, output: Union[Any, torch.Tensor, numbers.Number]) -> None:
         if not (isinstance(output, numbers.Number) or isinstance(output, torch.Tensor)):


Test patch:
diff --git a/tests/ignite/contrib/engines/test_common.py b/tests/ignite/contrib/engines/test_common.py
index dd82186e..5520403c 100644
--- a/tests/ignite/contrib/engines/test_common.py
+++ b/tests/ignite/contrib/engines/test_common.py
@@ -7,6 +7,7 @@ import torch.nn as nn
 from torch.utils.data.distributed import DistributedSampler
 
 import ignite.contrib.handlers as handlers
+import ignite.distributed as idist
 from ignite.contrib.engines.common import (
     _setup_logging,
     add_early_stopping_by_val_score,
@@ -73,7 +74,7 @@ def _test_setup_common_training_handlers(dirname, device, rank=0, local_rank=0,
         return loss
 
     train_sampler = None
-    if distributed:
+    if distributed and idist.get_world_size() > 1:
         train_sampler = MagicMock(spec=DistributedSampler)
         train_sampler.set_epoch = MagicMock()
 
@@ -126,9 +127,7 @@ def test_asserts_setup_common_training_handlers():
     ):
         setup_common_training_handlers(trainer, to_save={})
 
-    with pytest.warns(
-        UserWarning, match=r"Argument train_sampler is a distributed sampler, but no distributed setting detected"
-    ):
+    with pytest.warns(UserWarning, match=r"Argument train_sampler is a distributed sampler"):
         train_sampler = MagicMock(spec=DistributedSampler)
         setup_common_training_handlers(trainer, train_sampler=train_sampler)
 
@@ -351,7 +350,6 @@ def test_setup_tb_logging(dirname):
 
 
 def test_setup_visdom_logging(visdom_server):
-
     vis_logger = _test_setup_logging(
         setup_logging_fn=setup_visdom_logging,
         kwargs_dict={"server": visdom_server[0], "port": str(visdom_server[1])},
@@ -361,6 +359,7 @@ def test_setup_visdom_logging(visdom_server):
         with_optim=False,
     )
     vis_logger.close()
+
     vis_logger = _test_setup_logging(
         setup_logging_fn=setup_visdom_logging,
         kwargs_dict={"server": visdom_server[0], "port": str(visdom_server[1])},
diff --git a/tests/ignite/contrib/handlers/test_base_logger.py b/tests/ignite/contrib/handlers/test_base_logger.py
index bdc5da77..315b6de0 100644
--- a/tests/ignite/contrib/handlers/test_base_logger.py
+++ b/tests/ignite/contrib/handlers/test_base_logger.py
@@ -128,7 +128,6 @@ def test_attach_wrong_event_name():
 
 
 def test_attach_on_custom_event():
-
     n_epochs = 10
     data = list(range(150))
 
@@ -154,29 +153,32 @@ def test_attach_on_custom_event():
         mock_log_handler.assert_called_with(trainer, logger, event)
         assert mock_log_handler.call_count == n_calls
 
-    n_iterations = 10
-    cpe1 = CustomPeriodicEvent(n_iterations=n_iterations)
-    n = len(data) * n_epochs / n_iterations
-    nf = math.floor(n)
-    ns = nf + 1 if nf < n else nf
-    _test(cpe1.Events.ITERATIONS_10_STARTED, ns, cpe1)
-    _test(cpe1.Events.ITERATIONS_10_COMPLETED, nf, cpe1)
-
-    n_iterations = 15
-    cpe2 = CustomPeriodicEvent(n_iterations=n_iterations)
-    n = len(data) * n_epochs / n_iterations
-    nf = math.floor(n)
-    ns = nf + 1 if nf < n else nf
-    _test(cpe2.Events.ITERATIONS_15_STARTED, ns, cpe2)
-    _test(cpe2.Events.ITERATIONS_15_COMPLETED, nf, cpe2)
-
-    n_custom_epochs = 2
-    cpe3 = CustomPeriodicEvent(n_epochs=n_custom_epochs)
-    n = n_epochs / n_custom_epochs
-    nf = math.floor(n)
-    ns = nf + 1 if nf < n else nf
-    _test(cpe3.Events.EPOCHS_2_STARTED, ns, cpe3)
-    _test(cpe3.Events.EPOCHS_2_COMPLETED, nf, cpe3)
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        n_iterations = 10
+        cpe1 = CustomPeriodicEvent(n_iterations=n_iterations)
+        n = len(data) * n_epochs / n_iterations
+        nf = math.floor(n)
+        ns = nf + 1 if nf < n else nf
+        _test(cpe1.Events.ITERATIONS_10_STARTED, ns, cpe1)
+        _test(cpe1.Events.ITERATIONS_10_COMPLETED, nf, cpe1)
+
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        n_iterations = 15
+        cpe2 = CustomPeriodicEvent(n_iterations=n_iterations)
+        n = len(data) * n_epochs / n_iterations
+        nf = math.floor(n)
+        ns = nf + 1 if nf < n else nf
+        _test(cpe2.Events.ITERATIONS_15_STARTED, ns, cpe2)
+        _test(cpe2.Events.ITERATIONS_15_COMPLETED, nf, cpe2)
+
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        n_custom_epochs = 2
+        cpe3 = CustomPeriodicEvent(n_epochs=n_custom_epochs)
+        n = n_epochs / n_custom_epochs
+        nf = math.floor(n)
+        ns = nf + 1 if nf < n else nf
+        _test(cpe3.Events.EPOCHS_2_STARTED, ns, cpe3)
+        _test(cpe3.Events.EPOCHS_2_COMPLETED, nf, cpe3)
 
 
 def test_as_context_manager():
diff --git a/tests/ignite/contrib/handlers/test_custom_events.py b/tests/ignite/contrib/handlers/test_custom_events.py
index 9e9fb73d..c7f3543b 100644
--- a/tests/ignite/contrib/handlers/test_custom_events.py
+++ b/tests/ignite/contrib/handlers/test_custom_events.py
@@ -8,55 +8,51 @@ from ignite.engine import Engine
 
 def test_bad_input():
 
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_iterations="a")
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_iterations=0)
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_iterations=10.0)
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_epochs="a")
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_epochs=0)
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_epochs=10.0)
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent()
-
-    with pytest.raises(ValueError):
-        CustomPeriodicEvent(n_iterations=1, n_epochs=2)
+    with pytest.warns(DeprecationWarning, match=r"CustomPeriodicEvent is deprecated"):
+        with pytest.raises(ValueError, match="Argument n_iterations should be positive integer number"):
+            CustomPeriodicEvent(n_iterations="a")
+        with pytest.raises(ValueError, match="Argument n_iterations should be positive integer number"):
+            CustomPeriodicEvent(n_iterations=0)
+        with pytest.raises(ValueError, match="Argument n_iterations should be positive integer number"):
+            CustomPeriodicEvent(n_iterations=10.0)
+        with pytest.raises(ValueError, match="Argument n_epochs should be positive integer number"):
+            CustomPeriodicEvent(n_epochs="a")
+        with pytest.raises(ValueError, match="Argument n_epochs should be positive integer number"):
+            CustomPeriodicEvent(n_epochs=0)
+        with pytest.raises(ValueError, match="Argument n_epochs should be positive integer number"):
+            CustomPeriodicEvent(n_epochs=10.0)
+        with pytest.raises(ValueError, match="Either n_iterations or n_epochs should be defined"):
+            CustomPeriodicEvent()
+        with pytest.raises(ValueError, match="Either n_iterations or n_epochs should be defined"):
+            CustomPeriodicEvent(n_iterations=1, n_epochs=2)
 
 
 def test_new_events():
     def update(*args, **kwargs):
         pass
 
-    engine = Engine(update)
-    cpe = CustomPeriodicEvent(n_iterations=5)
-    cpe.attach(engine)
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        engine = Engine(update)
+        cpe = CustomPeriodicEvent(n_iterations=5)
+        cpe.attach(engine)
 
-    assert hasattr(cpe, "Events")
-    assert hasattr(cpe.Events, "ITERATIONS_5_STARTED")
-    assert hasattr(cpe.Events, "ITERATIONS_5_COMPLETED")
+        assert hasattr(cpe, "Events")
+        assert hasattr(cpe.Events, "ITERATIONS_5_STARTED")
+        assert hasattr(cpe.Events, "ITERATIONS_5_COMPLETED")
 
-    assert engine._allowed_events[-2] == getattr(cpe.Events, "ITERATIONS_5_STARTED")
-    assert engine._allowed_events[-1] == getattr(cpe.Events, "ITERATIONS_5_COMPLETED")
+        assert engine._allowed_events[-2] == getattr(cpe.Events, "ITERATIONS_5_STARTED")
+        assert engine._allowed_events[-1] == getattr(cpe.Events, "ITERATIONS_5_COMPLETED")
 
-    cpe = CustomPeriodicEvent(n_epochs=5)
-    cpe.attach(engine)
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        cpe = CustomPeriodicEvent(n_epochs=5)
+        cpe.attach(engine)
 
-    assert hasattr(cpe, "Events")
-    assert hasattr(cpe.Events, "EPOCHS_5_STARTED")
-    assert hasattr(cpe.Events, "EPOCHS_5_COMPLETED")
+        assert hasattr(cpe, "Events")
+        assert hasattr(cpe.Events, "EPOCHS_5_STARTED")
+        assert hasattr(cpe.Events, "EPOCHS_5_COMPLETED")
 
-    assert engine._allowed_events[-2] == getattr(cpe.Events, "EPOCHS_5_STARTED")
-    assert engine._allowed_events[-1] == getattr(cpe.Events, "EPOCHS_5_COMPLETED")
+        assert engine._allowed_events[-2] == getattr(cpe.Events, "EPOCHS_5_STARTED")
+        assert engine._allowed_events[-1] == getattr(cpe.Events, "EPOCHS_5_COMPLETED")
 
 
 def test_integration_iterations():
@@ -65,9 +61,9 @@ def test_integration_iterations():
             pass
 
         engine = Engine(update)
-
-        cpe = CustomPeriodicEvent(n_iterations=n_iterations)
-        cpe.attach(engine)
+        with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+            cpe = CustomPeriodicEvent(n_iterations=n_iterations)
+            cpe.attach(engine)
         data = list(range(n_iters_per_epoch))
 
         custom_period = [0]
@@ -114,8 +110,9 @@ def test_integration_epochs():
     engine = Engine(update)
 
     n_epochs = 3
-    cpe = CustomPeriodicEvent(n_epochs=n_epochs)
-    cpe.attach(engine)
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        cpe = CustomPeriodicEvent(n_epochs=n_epochs)
+        cpe.attach(engine)
     data = list(range(16))
 
     custom_period = [1]
diff --git a/tests/ignite/contrib/handlers/test_tqdm_logger.py b/tests/ignite/contrib/handlers/test_tqdm_logger.py
index 6649357c..46c14855 100644
--- a/tests/ignite/contrib/handlers/test_tqdm_logger.py
+++ b/tests/ignite/contrib/handlers/test_tqdm_logger.py
@@ -373,7 +373,8 @@ def test_pbar_on_custom_events(capsys):
 
     engine = Engine(update_fn)
     pbar = ProgressBar()
-    cpe = CustomPeriodicEvent(n_iterations=15)
+    with pytest.warns(DeprecationWarning, match="CustomPeriodicEvent is deprecated"):
+        cpe = CustomPeriodicEvent(n_iterations=15)
 
     with pytest.raises(ValueError, match=r"not in allowed events for this engine"):
         pbar.attach(engine, event_name=cpe.Events.ITERATIONS_15_COMPLETED, closing_event_name=Events.EPOCH_COMPLETED)
diff --git a/tests/ignite/contrib/handlers/test_trains_logger.py b/tests/ignite/contrib/handlers/test_trains_logger.py
index 21e6e433..58bbcec4 100644
--- a/tests/ignite/contrib/handlers/test_trains_logger.py
+++ b/tests/ignite/contrib/handlers/test_trains_logger.py
@@ -582,17 +582,18 @@ def test_integration(dirname):
 
     trainer = Engine(update_fn)
 
-    TrainsLogger.set_bypass_mode(True)
-    logger = TrainsLogger(output_uri=dirname)
+    with pytest.warns(UserWarning, match="TrainsSaver: running in bypass mode"):
+        TrainsLogger.set_bypass_mode(True)
+        logger = TrainsLogger(output_uri=dirname)
 
-    def dummy_handler(engine, logger, event_name):
-        global_step = engine.state.get_event_attrib_value(event_name)
-        logger.trains_logger.report_scalar(title="", series="", value="test_value", iteration=global_step)
+        def dummy_handler(engine, logger, event_name):
+            global_step = engine.state.get_event_attrib_value(event_name)
+            logger.trains_logger.report_scalar(title="", series="", value="test_value", iteration=global_step)
 
-    logger.attach(trainer, log_handler=dummy_handler, event_name=Events.EPOCH_COMPLETED)
+        logger.attach(trainer, log_handler=dummy_handler, event_name=Events.EPOCH_COMPLETED)
 
-    trainer.run(data, max_epochs=n_epochs)
-    logger.close()
+        trainer.run(data, max_epochs=n_epochs)
+        logger.close()
 
 
 def test_integration_as_context_manager(dirname):
@@ -606,28 +607,29 @@ def test_integration_as_context_manager(dirname):
     def update_fn(engine, batch):
         return next(losses_iter)
 
-    TrainsLogger.set_bypass_mode(True)
-    with TrainsLogger(output_uri=dirname) as trains_logger:
+    with pytest.warns(UserWarning, match="TrainsSaver: running in bypass mode"):
+        TrainsLogger.set_bypass_mode(True)
+        with TrainsLogger(output_uri=dirname) as trains_logger:
 
-        trainer = Engine(update_fn)
+            trainer = Engine(update_fn)
 
-        def dummy_handler(engine, logger, event_name):
-            global_step = engine.state.get_event_attrib_value(event_name)
-            logger.trains_logger.report_scalar(title="", series="", value="test_value", iteration=global_step)
+            def dummy_handler(engine, logger, event_name):
+                global_step = engine.state.get_event_attrib_value(event_name)
+                logger.trains_logger.report_scalar(title="", series="", value="test_value", iteration=global_step)
 
-        trains_logger.attach(trainer, log_handler=dummy_handler, event_name=Events.EPOCH_COMPLETED)
+            trains_logger.attach(trainer, log_handler=dummy_handler, event_name=Events.EPOCH_COMPLETED)
 
-        trainer.run(data, max_epochs=n_epochs)
+            trainer.run(data, max_epochs=n_epochs)
 
 
 def test_trains_disk_saver_integration():
     model = torch.nn.Module()
     to_save_serializable = {"model": model}
-
-    mock_logger = MagicMock(spec=TrainsLogger)
-    trains.Task.current_task = Mock(return_value=object())
-    trains_saver = TrainsSaver(mock_logger)
-    trains.binding.frameworks.WeightsFileHandler.create_output_model = MagicMock()
+    with pytest.warns(UserWarning, match="TrainsSaver created a temporary checkpoints directory"):
+        mock_logger = MagicMock(spec=TrainsLogger)
+        trains.Task.current_task = Mock(return_value=object())
+        trains_saver = TrainsSaver(mock_logger)
+        trains.binding.frameworks.WeightsFileHandler.create_output_model = MagicMock()
 
     checkpoint = Checkpoint(to_save=to_save_serializable, save_handler=trains_saver, n_saved=1)
 
@@ -648,12 +650,11 @@ def test_trains_disk_saver_integration_no_logger():
     model = torch.nn.Module()
     to_save_serializable = {"model": model}
 
-    trains.Task.current_task = Mock(return_value=object())
-    trains.binding.frameworks.WeightsFileHandler.create_output_model = MagicMock()
-
-    trains_saver = TrainsSaver()
-
-    checkpoint = Checkpoint(to_save=to_save_serializable, save_handler=trains_saver, n_saved=1)
+    with pytest.warns(UserWarning, match="TrainsSaver created a temporary checkpoints directory"):
+        trains.Task.current_task = Mock(return_value=object())
+        trains.binding.frameworks.WeightsFileHandler.create_output_model = MagicMock()
+        trains_saver = TrainsSaver()
+        checkpoint = Checkpoint(to_save=to_save_serializable, save_handler=trains_saver, n_saved=1)
 
     trainer = Engine(lambda e, b: None)
     trainer.state = State(epoch=0, iteration=0)
diff --git a/tests/ignite/contrib/handlers/test_visdom_logger.py b/tests/ignite/contrib/handlers/test_visdom_logger.py
index a2076108..758f2486 100644
--- a/tests/ignite/contrib/handlers/test_visdom_logger.py
+++ b/tests/ignite/contrib/handlers/test_visdom_logger.py
@@ -786,8 +786,11 @@ def test_grads_scalar_handler():
 
 def test_integration_no_server():
 
-    with pytest.raises(RuntimeError, match="Failed to connect to Visdom server"):
-        VisdomLogger()
+    with pytest.warns(
+        PendingDeprecationWarning, match="Visdom is eventually changing to default to raising exceptions"
+    ):
+        with pytest.raises(RuntimeError, match="Failed to connect to Visdom server"):
+            VisdomLogger()
 
 
 def test_logger_init_hostname_port(visdom_server):