Implemented changes proposed by code review.

Author: atollk

fs/base.py

@@ -22,7 +22,6 @@
 import six
 
 from . import copy, errors, fsencode, iotools, move, tools, walk, wildcard
-from .errors import Unsupported
 from .glob import BoundGlobber
 from .mode import validate_open_mode
 from .path import abspath, join, normpath
@@ -680,7 +679,7 @@ def readtext(
     gettext = _new_name(readtext, "gettext")
 
     def getmodified(self, path):
-        # type: (Text) -> datetime
+        # type: (Text) -> Optional[datetime]
         """Get the timestamp of the last modifying access of a resource.
 
         Arguments:
@@ -694,15 +693,9 @@ def getmodified(self, path):
         it might only have limited accuracy.
 
         """
-        timestamp = self.getinfo(path, ("details", "modified")).modified
-        if timestamp is None:
-            raise Unsupported(
-                "Last modified time is not supported by the filesystem"
-                + "for the requested resource: "
-                + path
-            )
-        else:
-            return timestamp
+        if self.getmeta().get("supports_mtime", False):
+            return self.getinfo(path, namespaces=["modified"]).modified
+        return self.getinfo(path, namespaces=["details"]).modified
 
     def getmeta(self, namespace="standard"):
         # type: (Text) -> Mapping[Text, object]

fs/ftpfs.py

@@ -13,10 +13,8 @@
 import typing
 from collections import OrderedDict
 from contextlib import contextmanager
-from datetime import datetime
 from ftplib import FTP
 
-from .time import epoch_to_datetime
 
 try:
     from ftplib import FTP_TLS
@@ -587,18 +585,6 @@ def create(self, path, wipe=False):
                 return True
         return False
 
-    def getmodified(self, path):
-        # type: (Text) -> datetime
-        if "MDTM" in self.features:
-            with self._lock:
-                with ftp_errors(self, path=path):
-                    cmd = "MDTM " + _encode(self.validatepath(path), self.ftp.encoding)
-                    response = self.ftp.sendcmd(cmd)
-                time_num = self._parse_ftp_time(response.split()[1])
-                if time_num is not None:
-                    return epoch_to_datetime(time_num)
-        return super(FTPFS, self).getmodified(path)
-
     @classmethod
     def _parse_ftp_time(cls, time_text):
         # type: (Text) -> Optional[int]
@@ -682,6 +668,18 @@ def getinfo(self, path, namespaces=None):
                 }
             )
 
+        if "modified" in namespaces:
+            if "basic" in namespaces or "details" in namespaces:
+                raise ValueError(
+                    'Cannot use the "modified" namespace in combination with others.'
+                )
+            with self._lock:
+                with ftp_errors(self, path=path):
+                    cmd = "MDTM " + _encode(self.validatepath(path), self.ftp.encoding)
+                    response = self.ftp.sendcmd(cmd)
+                modified_info = {"modified": self._parse_ftp_time(response.split()[1])}
+                return Info({"modified": modified_info})
+
         if self.supports_mlst:
             with self._lock:
                 with ftp_errors(self, path=path):

fs/info.py

@@ -317,9 +317,12 @@ def modified(self):
                 namespace is not in the Info.
 
         """
-        self._require_namespace("details")
-        _time = self._make_datetime(self.get("details", "modified"))
-        return _time
+        try:
+            self._require_namespace("details")
+            return self._make_datetime(self.get("details", "modified"))
+        except MissingInfoNamespace:
+            self._require_namespace("modified")
+            return self._make_datetime(self.get("modified", "modified"))
 
     @property
     def created(self):

fs/osfs.py

@@ -144,7 +144,7 @@ def __init__(
             "network": False,
             "read_only": False,
             "supports_rename": True,
-            "supports_mtime": True,
+            "supports_mtime": False,
             "thread_safe": True,
             "unicode_paths": os.path.supports_unicode_filenames,
             "virtual": False,

tests/test_memoryfs.py

@@ -69,7 +69,6 @@ def test_close_mem_free(self):
 
 
 class TestMemoryFile(unittest.TestCase):
-
     def setUp(self):
         self.fs = memoryfs.MemoryFS()
 

tests/test_opener.py

@@ -300,14 +300,26 @@ def test_user_data_opener(self, app_dir):
     def test_open_ftp(self, mock_FTPFS):
         open_fs("ftp://foo:[email protected]")
         mock_FTPFS.assert_called_once_with(
-            "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10, tls=False
+            "ftp.example.org",
+            passwd="bar",
+            port=21,
+            user="foo",
+            proxy=None,
+            timeout=10,
+            tls=False,
         )
 
     @mock.patch("fs.ftpfs.FTPFS")
     def test_open_ftps(self, mock_FTPFS):
         open_fs("ftps://foo:[email protected]")
         mock_FTPFS.assert_called_once_with(
-            "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10, tls=True
+            "ftp.example.org",
+            passwd="bar",
+            port=21,
+            user="foo",
+            proxy=None,
+            timeout=10,
+            tls=True,
         )
 
     @mock.patch("fs.ftpfs.FTPFS")

tests/test_wrap.py

@@ -177,7 +177,7 @@ def test_scandir(self):
         ]
         with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir:
             self.assertEqual(sorted(self.cached.scandir("/"), key=key), expected)
-            scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)])
+            scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)])
         with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir:
             self.assertEqual(sorted(self.cached.scandir("/"), key=key), expected)
             scandir.assert_not_called()
@@ -187,7 +187,7 @@ def test_isdir(self):
             self.assertTrue(self.cached.isdir("foo"))
             self.assertFalse(self.cached.isdir("egg"))  # is file
             self.assertFalse(self.cached.isdir("spam"))  # doesn't exist
-            scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)])
+            scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)])
         with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir:
             self.assertTrue(self.cached.isdir("foo"))
             self.assertFalse(self.cached.isdir("egg"))
@@ -199,7 +199,7 @@ def test_isfile(self):
             self.assertTrue(self.cached.isfile("egg"))
             self.assertFalse(self.cached.isfile("foo"))  # is dir
             self.assertFalse(self.cached.isfile("spam"))  # doesn't exist
-            scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)])
+            scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)])
         with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir:
             self.assertTrue(self.cached.isfile("egg"))
             self.assertFalse(self.cached.isfile("foo"))
@@ -211,7 +211,7 @@ def test_getinfo(self):
             self.assertEqual(self.cached.getinfo("foo"), self.fs.getinfo("foo"))
             self.assertEqual(self.cached.getinfo("/"), self.fs.getinfo("/"))
             self.assertNotFound(self.cached.getinfo, "spam")
-            scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)])
+            scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)])
         with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir:
             self.assertEqual(self.cached.getinfo("foo"), self.fs.getinfo("foo"))
             self.assertEqual(self.cached.getinfo("/"), self.fs.getinfo("/"))