Repository: pytorch/ignite

Issue:
MyPy: improve improve ignite.handlers module
## 🚀 Feature

Currently, mypy ignores all errors for all modules. We have to rework our typing such that mypy checks the code.
In this issue, let's improve https://github.com/pytorch/ignite/tree/master/ignite/handlers module such that mypy passes on it.

For Hacktoberfest contributors, feel free to ask questions for details if any and say that you would like to tackle the issue.
Please, take a look at CONTRIBUTING guide.


Discussion:
trsvchn: Hah... [`Found 5 errors in 3 files (checked 84 source files`)](https://travis-ci.org/github/pytorch/ignite/jobs/732630149#L281) on Travis, BUT
`Success: no issues found in 84 source files` on local machine... Interesting...
vfdev-5: @trsvchn I think it is the same issue as we discussed with @gruebel here : https://github.com/pytorch/ignite/pull/1352#discussion_r499236256
I merge his PR first and then we restart yours and it should be OK...
vfdev-5: @trsvchn there still something incorrect : https://travis-ci.org/github/pytorch/ignite/jobs/732921246#L1409
trsvchn: @vfdev-5 Oh, I see, it related to `base` module's `Serilizable` class:

```bash
ignite/handlers/early_stopping.py:46: error: Incompatible types in assignment
(expression has type "Tuple[str, str]", base class "Serializable" defined the type
as "Tuple[]")  [assignment]
        _state_dict_all_req_keys = (
                                   ^
ignite/handlers/checkpoint.py:235: error: Incompatible types in assignment
(expression has type "Tuple[str]", base class "Serializable" defined the type as
"Tuple[]")  [assignment]
        _state_dict_all_req_keys = ("saved",)
```
To fix we add need type type hints for `Serilizable` from `mixins`, related to #1342 

```python
class Serializable:

    _state_dict_all_req_keys = ()  # type: typle
    _state_dict_one_of_opt_keys = ()  # type: typle
```
vfdev-5: @trsvchn thanks for the update. I asked if assigned person will send a PR for that. If we have no answer in 6-12h, I can reassign it to you to fix it in a separate PR, if OK for you.
trsvchn: @vfdev-5 Sure! I would like to do that!
vfdev-5: @trsvchn sorry, your PR was accidentally closed by automations by the github
trsvchn: With the updated `mixins` now it runs smoothly:
```bash
Success: no issues found in 84 source files

The command "mypy --config-file mypy.ini" exited with 0.
```
trsvchn: @vfdev-5 Thank you for your guidelines and for your patience! :)

Patch:
diff --git a/ignite/handlers/checkpoint.py b/ignite/handlers/checkpoint.py
index 6870e1b0..6f4321b7 100644
--- a/ignite/handlers/checkpoint.py
+++ b/ignite/handlers/checkpoint.py
@@ -5,6 +5,7 @@ import tempfile
 import warnings
 from abc import ABCMeta, abstractmethod
 from collections import OrderedDict, namedtuple
+from tempfile import _TemporaryFileWrapper  # type: ignore
 from typing import Callable, Mapping, Optional, Union
 
 import torch
@@ -235,7 +236,7 @@ class Checkpoint(Serializable):
 
     def __init__(
         self,
-        to_save: Mapping,
+        to_save: Optional[Mapping],
         save_handler: Union[Callable, BaseSaveHandler],
         filename_prefix: str = "",
         score_function: Optional[Callable] = None,
@@ -287,7 +288,7 @@ class Checkpoint(Serializable):
         self.ext = "pt"
         self.global_step_transform = global_step_transform
         self.filename_pattern = filename_pattern
-        self._saved = []
+        self._saved = []  # type: list
         self.include_self = include_self
 
     @property
@@ -378,10 +379,11 @@ class Checkpoint(Serializable):
 
     def _setup_checkpoint(self) -> dict:
         checkpoint = {}
-        for k, obj in self.to_save.items():
-            if isinstance(obj, (nn.DataParallel, nn.parallel.DistributedDataParallel)):
-                obj = obj.module
-            checkpoint[k] = obj.state_dict()
+        if self.to_save is not None:
+            for k, obj in self.to_save.items():
+                if isinstance(obj, (nn.DataParallel, nn.parallel.DistributedDataParallel)):
+                    obj = obj.module
+                checkpoint[k] = obj.state_dict()
         return checkpoint
 
     @staticmethod
@@ -572,7 +574,7 @@ class DiskSaver(BaseSaveHandler):
         self._save_func(checkpoint, path, torch.save)
 
     def _save_xla(self, checkpoint: Mapping, path: str):
-        import torch_xla.core.xla_model as xm
+        import torch_xla.core.xla_model as xm  # type: ignore
 
         # all tpu procs should enter here as internally performs sync across device
         self._save_func(checkpoint, path, xm.save, rank=idist.get_rank())
@@ -582,8 +584,8 @@ class DiskSaver(BaseSaveHandler):
             func(checkpoint, path, **self.kwargs)
         else:
             tmp_file = None
-            tmp_name = None
-            tmp = None
+            tmp_name = ""
+            tmp = None  # type: _TemporaryFileWrapper
             if rank == 0:
                 tmp = tempfile.NamedTemporaryFile(delete=False, dir=self.dirname)
                 tmp_file = tmp.file
@@ -728,9 +730,15 @@ class ModelCheckpoint(Checkpoint):
     def last_checkpoint(self) -> Union[str, None]:
         if len(self._saved) < 1:
             return None
+
+        if not isinstance(self.save_handler, DiskSaver):
+            raise RuntimeError(
+                "Unable to save checkpoint, save_handler should be DiskSaver, got {}.".format(type(self.save_handler))
+            )
+
         return os.path.join(self.save_handler.dirname, self._saved[-1].filename)
 
-    def __call__(self, engine: Engine, to_save: Mapping) -> None:
+    def __call__(self, engine: Engine, to_save: Mapping) -> None:  # type: ignore
 
         if len(to_save) == 0:
             raise RuntimeError("No objects to checkpoint found.")
diff --git a/mypy.ini b/mypy.ini
index 24c1a726..2778b2e7 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -7,10 +7,6 @@ show_error_codes = True
 
 ignore_errors = True
 
-[mypy-ignite.handlers.*]
-
-ignore_errors = True
-
 [mypy-ignite.engine.*]
 
 ignore_errors = True


Test patch:
diff --git a/tests/ignite/handlers/test_checkpoint.py b/tests/ignite/handlers/test_checkpoint.py
index 6478ff49..11d044e2 100644
--- a/tests/ignite/handlers/test_checkpoint.py
+++ b/tests/ignite/handlers/test_checkpoint.py
@@ -563,6 +563,20 @@ def test_model_checkpoint_simple_recovery_from_existing_non_empty(dirname):
     _test(".pt", require_empty=False)
 
 
+def test_model_checkpoint_invalid_save_handler(dirname):
+    h = ModelCheckpoint(dirname, _PREFIX)
+    to_save = {"model": DummyModel()}
+    # Redefine save_handler
+    h.save_handler = lambda x, y: None
+    h(Engine(lambda x, y: None), to_save)
+
+    with pytest.raises(
+        RuntimeError,
+        match=r"Unable to save checkpoint, save_handler should be DiskSaver, got {}.".format(type(h.save_handler)),
+    ):
+        h.last_checkpoint
+
+
 def test_disk_saver_atomic(dirname):
 
     model = DummyModel()