Repository: pytorch/ignite

Issue:
TypeError: type object got multiple values for keyword argument 'logdir'
>from ignite.contrib.handlers.tensorboard_logger import TensorboardLogger
>tb = TensorboardLogger(log_dir=None) 

gives the following error:

>     self.file_writer = FileWriter(logdir=log_dir, **kwargs)
> TypeError: type object got multiple values for keyword argument 'logdir'

This also happens when I run

>from tensorboardX import SummaryWriter
>sw = SummaryWriter(log_dir=None)

>     self.file_writer = FileWriter(logdir=log_dir, **kwargs)
> TypeError: type object got multiple values for keyword argument 'logdir'

However, when I do don't put in logdir=log_dir, and simply pass in log_dir, it works fine.

I was looking to see if there's any conflicting reference with `self` or anything like that, but it doesn't seem like that's the problem, since all the `self`'s seem to be in place, so I'm not sure.

Any ideas?

ignite version: 1.1.0
tensorboard version: 1.13.1
tensorboardX version: 1.6

Discussion:
vfdev-5: @johnstill looks good to me! 
I think our `coverage` tool will complain about not covered change, so probably I'll need to provide a small test of that. I can do it later. But if you would like to contribute, it would be awesome!
Anyway, let's wait for CI and all other checks finished.
anmolsjoshi: @vfdev-5 @johnstill maybe we should add something in the docstring on how to access the log_dir? Mainly pointing out the difference between v1.7 and versions below

Some users might use tb_logger.writer.log_dir to get the directory name for ModelCheckpoint. 

Thoughts?
vfdev-5: @anmolsjoshi yes made we can just give a small note about that as it is up to TensorboardX API. 

> Some users might use tb_logger.writer.log_dir to get the directory name for ModelCheckpoint.

yes, it is the case of https://github.com/huggingface/transfer-learning-conv-ai
but a better way to do this IMO
```
path = ...
tb_logger = TensorboardLogger(path)
checkpoint = ModelCheckpoint(path)
```

anmolsjoshi: @vfdev-5 I think we're ok without it. I'm good with this merge. 

Patch:
diff --git a/ignite/contrib/handlers/tensorboard_logger.py b/ignite/contrib/handlers/tensorboard_logger.py
index 938113ee..b2c9256a 100644
--- a/ignite/contrib/handlers/tensorboard_logger.py
+++ b/ignite/contrib/handlers/tensorboard_logger.py
@@ -405,7 +405,16 @@ class TensorboardLogger(BaseLogger):
             raise RuntimeError("This contrib module requires tensorboardX to be installed. "
                                "Please install it with command: \n pip install tensorboardX")
 
-        self.writer = SummaryWriter(logdir=log_dir)
+        try:
+            self.writer = SummaryWriter(logdir=log_dir)
+        except TypeError as err:
+            if "type object got multiple values for keyword argument 'logdir'" == str(err):
+                self.writer = SummaryWriter(log_dir=log_dir)
+                warnings.warn('tensorboardX version < 1.7 will not be supported '
+                              'after ignite 0.3.0; please upgrade',
+                              DeprecationWarning)
+            else:
+                raise err
 
     def close(self):
         self.writer.close()


Test patch:
diff --git a/tests/ignite/contrib/handlers/test_tensorboard_logger.py b/tests/ignite/contrib/handlers/test_tensorboard_logger.py
index ce2588e3..0ada180d 100644
--- a/tests/ignite/contrib/handlers/test_tensorboard_logger.py
+++ b/tests/ignite/contrib/handlers/test_tensorboard_logger.py
@@ -3,10 +3,9 @@ import tempfile
 import shutil
 import math
 
-import numpy as np
 import pytest
 
-from mock import MagicMock, call, ANY
+from mock import MagicMock, call, ANY, Mock
 
 import torch
 
@@ -541,3 +540,40 @@ def test_no_tensorboardX(dirname, no_site_packages):
 
     with pytest.raises(RuntimeError, match=r"This contrib module requires tensorboardX to be installed"):
         TensorboardLogger(log_dir=dirname)
+
+
+@pytest.fixture
+def mock_tb_module():
+    import sys
+    import types
+
+    module_name = 'tensorboardX'
+    tb_module = types.ModuleType(module_name)
+    prev_tb_module = sys.modules[module_name]
+    sys.modules[module_name] = tb_module
+
+    yield tb_module
+    sys.modules[module_name] = prev_tb_module
+
+
+def test_init_tb1p6(mock_tb_module):
+
+    def side_effect_v1p6(*args, **kwargs):
+        if 'logdir' in kwargs:
+            raise TypeError("type object got multiple values for keyword argument 'logdir'")
+
+    mock_tb_module.SummaryWriter = Mock(name='tensorboardX.SummaryWriter', side_effect=side_effect_v1p6)
+
+    with pytest.warns(DeprecationWarning, match=r'tensorboardX version < 1.7 will not be supported'):
+        TensorboardLogger(log_dir=None)
+
+
+def test_init_typeerror_exception(mock_tb_module):
+
+    def side_effect(*args, **kwargs):
+        raise TypeError("a problem")
+
+    mock_tb_module.SummaryWriter = Mock(name='tensorboardX.SummaryWriter', side_effect=side_effect)
+
+    with pytest.raises(TypeError, match=r'a problem'):
+        TensorboardLogger(log_dir=None)