From 464cb9224a6009d834dc02bcc099a6fcf721173e Mon Sep 17 00:00:00 2001 From: Luis Antonio Obis Aparicio Date: Mon, 13 Nov 2023 14:30:09 -0600 Subject: [PATCH 1/6] sort mode string --- fsspec/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fsspec/core.py b/fsspec/core.py index a1e15b2eb..9a42bf6f9 100644 --- a/fsspec/core.py +++ b/fsspec/core.py @@ -99,6 +99,8 @@ def __repr__(self): def __enter__(self): mode = self.mode.replace("t", "").replace("b", "") + "b" + sorting_order = {"r": 0, "w": 1, "a": 2, "b": 3, "+": 4} + mode = "".join(sorted(mode, key=lambda c: sorting_order.get(c, float("inf")))) f = self.fs.open(self.path, mode=mode) From a7e9d68b7eabcd4591e7cac0e92200e4a0553cb7 Mon Sep 17 00:00:00 2001 From: Luis Antonio Obis Aparicio Date: Mon, 13 Nov 2023 14:41:23 -0600 Subject: [PATCH 2/6] test different open modes --- fsspec/implementations/tests/test_memory.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/fsspec/implementations/tests/test_memory.py b/fsspec/implementations/tests/test_memory.py index 05a40b287..519f83a08 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", "bw", "w", "wb", "ab", "ba", "rb+", "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) From fa37f052936da6811fabeb8931ce59176786a1d8 Mon Sep 17 00:00:00 2001 From: Luis Antonio Obis Aparicio Date: Mon, 13 Nov 2023 16:42:49 -0600 Subject: [PATCH 3/6] use "r+b" instead of "rb+" mode --- fsspec/caching.py | 2 +- fsspec/implementations/cached.py | 4 ++-- fsspec/implementations/http.py | 2 +- fsspec/implementations/memory.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/fsspec/caching.py b/fsspec/caching.py index 2812724fc..8d441fe46 100644 --- a/fsspec/caching.py +++ b/fsspec/caching.py @@ -111,7 +111,7 @@ def _makefile(self) -> mmap.mmap | bytearray: 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 89fab0791..eef916bb2 100644 --- a/fsspec/implementations/cached.py +++ b/fsspec/implementations/cached.py @@ -771,10 +771,10 @@ def __init__(self, fs, path, fn=None, mode="wb", autocommit=True, seek=0): 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 cdd84c5ce..7b249bd94 100644 --- a/fsspec/implementations/http.py +++ b/fsspec/implementations/http.py @@ -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 32daaf0f1..4022ff13a 100644 --- a/fsspec/implementations/memory.py +++ b/fsspec/implementations/memory.py @@ -175,7 +175,7 @@ def _open( 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": From e2a57bebc7c920e856f57aacf9d453f0433e5617 Mon Sep 17 00:00:00 2001 From: Luis Antonio Obis Aparicio Date: Mon, 13 Nov 2023 16:43:31 -0600 Subject: [PATCH 4/6] revert normalizing file mode --- fsspec/core.py | 96 ++++++++++++++++++++++++-------------------------- 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/fsspec/core.py b/fsspec/core.py index 9a42bf6f9..b3b133138 100644 --- a/fsspec/core.py +++ b/fsspec/core.py @@ -62,14 +62,14 @@ class OpenFile: """ def __init__( - self, - fs, - path, - mode="rb", - compression=None, - encoding=None, - errors=None, - newline=None, + self, + fs, + path, + mode="rb", + compression=None, + encoding=None, + errors=None, + newline=None, ): self.fs = fs self.path = path @@ -99,8 +99,6 @@ def __repr__(self): def __enter__(self): mode = self.mode.replace("t", "").replace("b", "") + "b" - sorting_order = {"r": 0, "w": 1, "a": 2, "b": 3, "+": 4} - mode = "".join(sorted(mode, key=lambda c: sorting_order.get(c, float("inf")))) f = self.fs.open(self.path, mode=mode) @@ -204,18 +202,18 @@ def __repr__(self): def open_files( - urlpath, - mode="rb", - compression=None, - encoding="utf8", - errors=None, - name_function=None, - num=1, - protocol=None, - newline=None, - auto_mkdir=True, - expand=True, - **kwargs, + urlpath, + mode="rb", + compression=None, + encoding="utf8", + errors=None, + name_function=None, + num=1, + protocol=None, + newline=None, + auto_mkdir=True, + expand=True, + **kwargs, ): """Given a path or paths, return a list of ``OpenFile`` objects. @@ -339,8 +337,8 @@ def _un_chain(path, kwargs): kw = dict(**extra_kwargs, **kws) bit = cls._strip_protocol(bit) if ( - protocol in {"blockcache", "filecache", "simplecache"} - and "target_protocol" not in kw + protocol in {"blockcache", "filecache", "simplecache"} + and "target_protocol" not in kw ): bit = previous_bit out.append((bit, protocol, kw)) @@ -399,14 +397,14 @@ def url_to_fs(url, **kwargs): def open( - urlpath, - mode="rb", - compression=None, - encoding="utf8", - errors=None, - protocol=None, - newline=None, - **kwargs, + urlpath, + mode="rb", + compression=None, + encoding="utf8", + errors=None, + protocol=None, + newline=None, + **kwargs, ): """Given a path or paths, return one ``OpenFile`` object. @@ -475,9 +473,9 @@ def open( def open_local( - url: str | list[str] | Path | list[Path], - mode: str = "rb", - **storage_options: dict, + url: str | list[str] | Path | list[Path], + mode: str = "rb", + **storage_options: dict, ) -> str | list[str]: """Open file(s) which can be resolved to local @@ -583,13 +581,13 @@ def expand_paths_if_needed(paths, mode, num, fs, name_function): def get_fs_token_paths( - urlpath, - mode="rb", - num=1, - name_function=None, - storage_options=None, - protocol=None, - expand=True, + urlpath, + mode="rb", + num=1, + name_function=None, + storage_options=None, + protocol=None, + expand=True, ): """Filesystem, deterministic token, and paths from a urlpath and options. @@ -697,13 +695,13 @@ class PickleableTextIOWrapper(io.TextIOWrapper): """ def __init__( - self, - buffer, - encoding=None, - errors=None, - newline=None, - line_buffering=False, - write_through=False, + self, + buffer, + encoding=None, + errors=None, + newline=None, + line_buffering=False, + write_through=False, ): self.args = buffer, encoding, errors, newline, line_buffering, write_through super().__init__(*self.args) From de0c272e4da4d733f72e5486a4efcd8f5fc7f4cc Mon Sep 17 00:00:00 2001 From: Luis Antonio Obis Aparicio Date: Mon, 13 Nov 2023 16:43:56 -0600 Subject: [PATCH 5/6] format --- fsspec/core.py | 94 +++++++++++++++++++++++++------------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/fsspec/core.py b/fsspec/core.py index b3b133138..a1e15b2eb 100644 --- a/fsspec/core.py +++ b/fsspec/core.py @@ -62,14 +62,14 @@ class OpenFile: """ def __init__( - self, - fs, - path, - mode="rb", - compression=None, - encoding=None, - errors=None, - newline=None, + self, + fs, + path, + mode="rb", + compression=None, + encoding=None, + errors=None, + newline=None, ): self.fs = fs self.path = path @@ -202,18 +202,18 @@ def __repr__(self): def open_files( - urlpath, - mode="rb", - compression=None, - encoding="utf8", - errors=None, - name_function=None, - num=1, - protocol=None, - newline=None, - auto_mkdir=True, - expand=True, - **kwargs, + urlpath, + mode="rb", + compression=None, + encoding="utf8", + errors=None, + name_function=None, + num=1, + protocol=None, + newline=None, + auto_mkdir=True, + expand=True, + **kwargs, ): """Given a path or paths, return a list of ``OpenFile`` objects. @@ -337,8 +337,8 @@ def _un_chain(path, kwargs): kw = dict(**extra_kwargs, **kws) bit = cls._strip_protocol(bit) if ( - protocol in {"blockcache", "filecache", "simplecache"} - and "target_protocol" not in kw + protocol in {"blockcache", "filecache", "simplecache"} + and "target_protocol" not in kw ): bit = previous_bit out.append((bit, protocol, kw)) @@ -397,14 +397,14 @@ def url_to_fs(url, **kwargs): def open( - urlpath, - mode="rb", - compression=None, - encoding="utf8", - errors=None, - protocol=None, - newline=None, - **kwargs, + urlpath, + mode="rb", + compression=None, + encoding="utf8", + errors=None, + protocol=None, + newline=None, + **kwargs, ): """Given a path or paths, return one ``OpenFile`` object. @@ -473,9 +473,9 @@ def open( def open_local( - url: str | list[str] | Path | list[Path], - mode: str = "rb", - **storage_options: dict, + url: str | list[str] | Path | list[Path], + mode: str = "rb", + **storage_options: dict, ) -> str | list[str]: """Open file(s) which can be resolved to local @@ -581,13 +581,13 @@ def expand_paths_if_needed(paths, mode, num, fs, name_function): def get_fs_token_paths( - urlpath, - mode="rb", - num=1, - name_function=None, - storage_options=None, - protocol=None, - expand=True, + urlpath, + mode="rb", + num=1, + name_function=None, + storage_options=None, + protocol=None, + expand=True, ): """Filesystem, deterministic token, and paths from a urlpath and options. @@ -695,13 +695,13 @@ class PickleableTextIOWrapper(io.TextIOWrapper): """ def __init__( - self, - buffer, - encoding=None, - errors=None, - newline=None, - line_buffering=False, - write_through=False, + self, + buffer, + encoding=None, + errors=None, + newline=None, + line_buffering=False, + write_through=False, ): self.args = buffer, encoding, errors, newline, line_buffering, write_through super().__init__(*self.args) From 4220028022e3ffc01a2e7f04050854f401e24d38 Mon Sep 17 00:00:00 2001 From: Luis Antonio Obis Aparicio Date: Mon, 13 Nov 2023 16:45:02 -0600 Subject: [PATCH 6/6] update test --- fsspec/implementations/tests/test_memory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsspec/implementations/tests/test_memory.py b/fsspec/implementations/tests/test_memory.py index 519f83a08..038d72eaa 100644 --- a/fsspec/implementations/tests/test_memory.py +++ b/fsspec/implementations/tests/test_memory.py @@ -192,7 +192,7 @@ def test_seekable(m): # https://github.com/fsspec/filesystem_spec/issues/1425 -@pytest.mark.parametrize("mode", ["r", "rb", "bw", "w", "wb", "ab", "ba", "rb+", "r+b"]) +@pytest.mark.parametrize("mode", ["r", "rb", "w", "wb", "ab", "r+b"]) def test_open_mode(m, mode): filename = "mode.txt" m.touch(filename)