Fix support for symlink save_dir in TensorBoardLogger (Lightning-AI#6730)

* Add test for symlink support and initial fix

* Respond to comment and add docstring

* Update CHANGELOG.md

* Simplify

* Update pytorch_lightning/utilities/cloud_io.py

Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>

* Make `LightningLocalFileSystem` protected

Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>

Author: 2 people

CHANGELOG.md

@@ -40,6 +40,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 - Fixed resolve a bug with omegaconf and xm.save ([#6741](https://github.com/PyTorchLightning/pytorch-lightning/pull/6741))
 
+- Fixed a bug where `TensorBoardLogger` would give a warning and not log correctly to a symbolic link `save_dir` ([#6730](https://github.com/PyTorchLightning/pytorch-lightning/pull/6730))
+
+
 ## [1.2.4] - 2021-03-16
 
 ### Changed

pytorch_lightning/utilities/cloud_io.py

@@ -12,15 +12,29 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import io
 from distutils.version import LooseVersion
 from pathlib import Path
 from typing import IO, Union
 
 import fsspec
+from fsspec.implementations.local import LocalFileSystem
+
 import torch
 
 
+class _LightningLocalFileSystem(LocalFileSystem):
+    """Extension of ``fsspec.implementations.local.LocalFileSystem`` where ``LightningLocalFileSystem.isdir`` behaves
+    the same as ``os.isdir``.
+
+    To be removed when https://github.com/intake/filesystem_spec/issues/591 is fixed.
+    """
+
+    def isdir(self, path: str) -> bool:
+        return os.path.isdir(path)  # follows symlinks
+
+
 def load(path_or_url: Union[str, IO, Path], map_location=None):
     if not isinstance(path_or_url, (str, Path)):
         # any sort of BytesIO or similiar
@@ -39,7 +53,7 @@ def get_filesystem(path: Union[str, Path]):
         return fsspec.filesystem(path.split(":", 1)[0])
     else:
         # use local filesystem
-        return fsspec.filesystem("file")
+        return _LightningLocalFileSystem()
 
 
 def atomic_save(checkpoint, filepath: str):

tests/loggers/test_tensorboard.py

@@ -303,3 +303,22 @@ def test_tensorboard_save_hparams_to_yaml_once(tmpdir):
     hparams_file = "hparams.yaml"
     assert os.path.isfile(os.path.join(trainer.log_dir, hparams_file))
     assert not os.path.isfile(os.path.join(tmpdir, hparams_file))
+
+
+@mock.patch('pytorch_lightning.loggers.tensorboard.log')
+def test_tensorboard_with_symlink(log, tmpdir):
+    """
+    Tests a specific failure case when tensorboard logger is used with empty name, symbolic link ``save_dir``, and
+    relative paths.
+    """
+    os.chdir(tmpdir)  # need to use relative paths
+    source = os.path.join('.', 'lightning_logs')
+    dest = os.path.join('.', 'sym_lightning_logs')
+
+    os.makedirs(source, exist_ok=True)
+    os.symlink(source, dest)
+
+    logger = TensorBoardLogger(save_dir=dest, name='')
+    _ = logger.version
+
+    log.warning.assert_not_called()