Repository: fsspec/filesystem_spec
Pull request #1422 (2023-11-10 15:22:39)

Pass `kwargs` to `info` and `exists` calls in `glob`
Same as `find`, `info` and `exists` propagate the `kwargs` values to `ls`, so we should also pass the `kwargs` values in `glob` to them to be consistent.

(We have a use case for this in the `HfFileSystem`: https://github.com/huggingface/huggingface_hub/pull/1815) 


Patch:
diff --git a/fsspec/asyn.py b/fsspec/asyn.py
index ee71c35..d19836a 100644
--- a/fsspec/asyn.py
+++ b/fsspec/asyn.py
@@ -672,9 +672,9 @@ class AsyncFileSystem(AbstractFileSystem):
             [self._size(p) for p in paths], batch_size=batch_size
         )
 
-    async def _exists(self, path):
+    async def _exists(self, path, **kwargs):
         try:
-            await self._info(path)
+            await self._info(path, **kwargs)
             return True
         except FileNotFoundError:
             return False
@@ -760,11 +760,11 @@ class AsyncFileSystem(AbstractFileSystem):
         detail = kwargs.pop("detail", False)
 
         if not has_magic(path):
-            if await self._exists(path):
+            if await self._exists(path, **kwargs):
                 if not detail:
                     return [path]
                 else:
-                    return {path: await self._info(path)}
+                    return {path: await self._info(path, **kwargs)}
             else:
                 if not detail:
                     return []  # glob of non-existent returns empty
diff --git a/fsspec/caching.py b/fsspec/caching.py
index 2812724..8d441fe 100644
--- a/fsspec/caching.py
+++ b/fsspec/caching.py
@@ -111,7 +111,7 @@ class MMapCache(BaseCache):
             fd.write(b"1")
             fd.flush()
         else:
-            fd = open(self.location, "rb+")
+            fd = open(self.location, "r+b")
 
         return mmap.mmap(fd.fileno(), self.size)
 
diff --git a/fsspec/implementations/cached.py b/fsspec/implementations/cached.py
index 89fab07..eef916b 100644
--- a/fsspec/implementations/cached.py
+++ b/fsspec/implementations/cached.py
@@ -771,10 +771,10 @@ class LocalTempFile:
         self.autocommit = autocommit
 
     def __reduce__(self):
-        # always open in rb+ to allow continuing writing at a location
+        # always open in r+b to allow continuing writing at a location
         return (
             LocalTempFile,
-            (self.fs, self.path, self.fn, "rb+", self.autocommit, self.tell()),
+            (self.fs, self.path, self.fn, "r+b", self.autocommit, self.tell()),
         )
 
     def __enter__(self):
diff --git a/fsspec/implementations/http.py b/fsspec/implementations/http.py
index cdd84c5..cf759c4 100644
--- a/fsspec/implementations/http.py
+++ b/fsspec/implementations/http.py
@@ -458,11 +458,11 @@ class HTTPFileSystem(AsyncFileSystem):
         detail = kwargs.pop("detail", False)
 
         if not has_magic(path):
-            if await self._exists(path):
+            if await self._exists(path, **kwargs):
                 if not detail:
                     return [path]
                 else:
-                    return {path: await self._info(path)}
+                    return {path: await self._info(path, **kwargs)}
             else:
                 if not detail:
                     return []  # glob of non-existent returns empty
@@ -802,7 +802,7 @@ async def get_range(session, url, start, end, file=None, **kwargs):
     async with r:
         out = await r.read()
     if file:
-        with open(file, "rb+") as f:
+        with open(file, "r+b") as f:
             f.seek(start)
             f.write(out)
     else:
diff --git a/fsspec/implementations/memory.py b/fsspec/implementations/memory.py
index 32daaf0..4022ff1 100644
--- a/fsspec/implementations/memory.py
+++ b/fsspec/implementations/memory.py
@@ -175,7 +175,7 @@ class MemoryFileSystem(AbstractFileSystem):
             parent = self._parent(parent)
             if self.isfile(parent):
                 raise FileExistsError(parent)
-        if mode in ["rb", "ab", "rb+"]:
+        if mode in ["rb", "ab", "r+b"]:
             if path in self.store:
                 f = self.store[path]
                 if mode == "ab":
diff --git a/fsspec/spec.py b/fsspec/spec.py
index dffbeb8..f3f4123 100644
--- a/fsspec/spec.py
+++ b/fsspec/spec.py
@@ -110,6 +110,7 @@ class AbstractFileSystem(metaclass=_Cached):
     async_impl = False
     mirror_sync_methods = False
     root_marker = ""  # For some FSs, may require leading '/' or other character
+    transaction_type = Transaction
 
     #: Extra *class attributes* that should be considered when hashing.
     _extra_tokenize_attributes = ()
@@ -236,13 +237,13 @@ class AbstractFileSystem(metaclass=_Cached):
         for the normal and exception cases.
         """
         if self._transaction is None:
-            self._transaction = Transaction(self)
+            self._transaction = self.transaction_type(self)
         return self._transaction
 
     def start_transaction(self):
         """Begin write transaction for deferring files, non-context version"""
         self._intrans = True
-        self._transaction = Transaction(self)
+        self._transaction = self.transaction_type(self)
         return self.transaction
 
     def end_transaction(self):
@@ -574,11 +575,11 @@ class AbstractFileSystem(metaclass=_Cached):
         detail = kwargs.pop("detail", False)
 
         if not has_magic(path):
-            if self.exists(path):
+            if self.exists(path, **kwargs):
                 if not detail:
                     return [path]
                 else:
-                    return {path: self.info(path)}
+                    return {path: self.info(path, **kwargs)}
             else:
                 if not detail:
                     return []  # glob of non-existent returns empty
diff --git a/fsspec/transaction.py b/fsspec/transaction.py
index d0b5dc9..67584d6 100644
--- a/fsspec/transaction.py
+++ b/fsspec/transaction.py
@@ -1,3 +1,6 @@
+from collections import deque
+
+
 class Transaction:
     """Filesystem transaction write context
 
@@ -13,10 +16,11 @@ class Transaction:
         fs: FileSystem instance
         """
         self.fs = fs
-        self.files = []
+        self.files = deque()
 
     def __enter__(self):
         self.start()
+        return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         """End transaction and commit, if exit is not due to exception"""
@@ -27,17 +31,17 @@ class Transaction:
 
     def start(self):
         """Start a transaction on this FileSystem"""
-        self.files = []  # clean up after previous failed completions
+        self.files = deque()  # clean up after previous failed completions
         self.fs._intrans = True
 
     def complete(self, commit=True):
         """Finish transaction: commit or discard all deferred files"""
-        for f in self.files:
+        while self.files:
+            f = self.files.popleft()
             if commit:
                 f.commit()
             else:
                 f.discard()
-        self.files = []
         self.fs._intrans = False
 
 


Test patch:
diff --git a/fsspec/implementations/tests/test_memory.py b/fsspec/implementations/tests/test_memory.py
index 05a40b2..038d72e 100644
--- a/fsspec/implementations/tests/test_memory.py
+++ b/fsspec/implementations/tests/test_memory.py
@@ -191,6 +191,15 @@ def test_seekable(m):
     assert f.tell() == 2
 
 
+# https://github.com/fsspec/filesystem_spec/issues/1425
+@pytest.mark.parametrize("mode", ["r", "rb", "w", "wb", "ab", "r+b"])
+def test_open_mode(m, mode):
+    filename = "mode.txt"
+    m.touch(filename)
+    with m.open(filename, mode=mode) as _:
+        pass
+
+
 def test_remove_all(m):
     m.touch("afile")
     m.rm("/", recursive=True)