Repository: fsspec/filesystem_spec
Pull request #1567 (2024-04-07 20:47:26)

LocalFileSystem restore _strip_protocol signature
Hello,

I noticed that the `LocalFileSystem._strip_protocol` signature changed in #1477, when running the universal_pathlib test-suite against the current fsspec version.

To me it seems that the intention of #1477 was initially to prevent `"C:/"` or `"C:\\"` to be reduced to `"C:"` by `_strip_protocol`, but in addition it introduced function signature changes in `fsspec.implementations.local` and minor changes in `fsspec.mapping`.

I created this PR as a basis for discussion if the signature change could be avoided. In this PR I reverted the changes to `LocalFileSystem` and `make_path_posix` but kept the changes to the tests (first commit) and then provide an alternative implementation that avoids the function signature change.

I would also be happy to try to add more tests around the `LocalFileSystem._parent`, `LocalFileSystem._strip_protocol` and `make_path_posix` behavior for edge-cases if there is interest. But windows is not my main OS for daily work, so I am very likely not aware of most of them.


Cheers,
Andreas






martindurant: @fleming79 , this undoes much of your code, but maintains the tests for correctness, while reimplementing consistency for _strip_protocol. Do you have thoughts?
fleming79: I have no issue with removing the option 'remove_trailing_slash' if it isn't required. It was provided because there were `.rstrip('/')` calls scattered about.

The question I pose is - which code is more maintainable/performant/correct?
 


ap--: Thanks for the quick response!

Regarding maintainability, I would prefer if the method signatures would agree with `AbstractFileSystem`. For example, enforcing #1104 (where possible) would make downstream development easier.

Regarding performance, I'm not a big fan of micro-benchmarks anymore, but I ran one (checking 10000 generated URIs) and the implementation in this PR seems to be equally fast/slow (within margin of error) as `2024.3.1` and `2024.3.0` for `_strip_protocol`. It is ~2% slower for `_parent`. This was on an M2max arm macbook, only tested with `py3.11` and only run a few times. Variation between runs is larger than the differences within a run.

<details>
<summary>[click to expand] benchmark for `_strip_protocol` and `_parent`</summary>

```
❯ asv run
· Creating environments
· Discovering benchmarks
·· Uninstalling from virtualenv-py3.11
·· Building 518984d4 <master> for virtualenv-py3.11...
·· Installing 518984d4 <master> into virtualenv-py3.11.
· Running 6 total benchmarks (3 commits * 1 environments * 2 benchmarks)
[ 0.00%] · For filesystem_spec commit 518984d4 <master>:
[ 0.00%] ·· Benchmarking virtualenv-py3.11
[ 8.33%] ··· Running (bench_strip_protocol.Suite.time_parent--)..
[25.00%] ··· bench_strip_protocol.Suite.time_parent                        207±0.8ms
[33.33%] ··· bench_strip_protocol.Suite.time_split_protocol                  205±2ms
[33.33%] · For filesystem_spec commit 4bd16f64 <2024.3.0^0>:
[33.33%] ·· Building for virtualenv-py3.11.
[33.33%] ·· Benchmarking virtualenv-py3.11
[41.67%] ··· Running (bench_strip_protocol.Suite.time_parent--)..
[58.33%] ··· bench_strip_protocol.Suite.time_parent                        207±0.7ms
[66.67%] ··· bench_strip_protocol.Suite.time_split_protocol                  204±2ms
[66.67%] · For filesystem_spec commit c9eaf46e <local-fs-restore-strip-protocol>:
[66.67%] ·· Building for virtualenv-py3.11....
[66.67%] ·· Benchmarking virtualenv-py3.11
[75.00%] ··· Running (bench_strip_protocol.Suite.time_parent--)..
[91.67%] ··· bench_strip_protocol.Suite.time_parent                          211±1ms
[100.00%] ··· bench_strip_protocol.Suite.time_split_protocol                 204±2ms
```


```python
# pip install asv

import itertools
import random
import string
from fsspec import get_filesystem_class

random.seed(0)

def _make_random_path():
    # build a random length path
    pth_len = random.randint(5, 40)
    return "".join(random.sample(string.ascii_letters + "/", k=pth_len))

def _make_uris(n):
    # create uris with and without protocols and with and without netlocs
    it_proto_netloc = itertools.cycle(
        itertools.product(
            ["file", "local", "wrong", None],
            ["netloc", ""]
        )
    )

    for _ in range(n):
        proto, netloc = next(it_proto_netloc)
        pth = _make_random_path()
        if proto and netloc:
            yield f"{proto}://{netloc}/{pth}"
        elif proto:
            yield f"{proto}:/{pth}"
        else:
            yield f"{netloc}/{pth}"

uris = list(_make_uris(10000))


class Suite:

    def setup(self):
        self.fs_cls = get_filesystem_class("file")
        self.uris = uris

    def teardown(self):
        del self.uris

    def time_split_protocol(self):
        for uri in self.uris:
            self.fs_cls._strip_protocol(uri)

    def time_parent(self):
        for uri in self.uris:
            self.fs_cls._parent(uri)
```

</details>


To be able to answer if this implementation is more or less correct would require more tests. I'll try to find some time to come up with more cases for the test-suite.

Cheers,
Andreas
martindurant: I don't think we can trust timing differences <5%; in these cases it can even matter which order you run them (whether the CPU is warm, etc). The interesting thing would be to check on Windows (but they have lots of path styles). 
fleming79: The original `make_path_posix` contains quite a lot of strip protocol code in it. It also includes 'sep' as an argument implying it can work for an alternate os. 
I'd prefer to see the newer code tweaked to remove the signature.

Here is a starting point.

```




def make_path_posix(path):
    """Make path generic for current OS"""
    if not isinstance(path, str):
        if isinstance(path, (list, set, tuple)):
            return type(path)(make_path_posix(p) for p in path)
        else:
            path = str(stringify_path(path))
    if os.sep == "/":
        # Native posix
        if path.startswith("/"):
            # most common fast case for posix
            return path.rstrip("/") or "/"
        elif path.startswith("~"):
            return make_path_posix(osp.expanduser(path))
        elif path.startswith("./"):
            path = path[2:]
            path = f"{os.getcwd()}/{path}"
            return path.rstrip("/") or "/"
        return f"{os.getcwd()}/{path}"
    else:
        # NT handling
        if len(path) > 1:
            if path[1] == ":":
                # windows full path like "C:\\local\\path"
                if len(path) <= 3:
                    # nt root (something like c:/)
                    return path[0] + ":/"
                path = path.replace("\\", "/").replace("//", "/")
                return path
            elif path[0] == "~":
                return make_path_posix(osp.expanduser(path))
            elif path.startswith(("\\\\", "//")):
                # windows UNC/DFS-style paths
                return "//" + path[2:].replace("\\", "/").replace("//", "/")
        return make_path_posix(osp.abspath(path))


```

```

    @classmethod
    def _strip_protocol(cls, path):
        path = stringify_path(path)
        if path.startswith(("file:",'local')):
            if os.sep == '\\' and path.startswith("file:///"):
                path = path[8:]
            elif path.startswith("file://"):
                path = path[7:]
            elif path.startswith("file:"):
                path = path[5:]
            elif path.startswith("local://"):
                path = path[8:]
            elif path.startswith("local:"):
                path = path[6:]
        drive, path = osp.splitdrive(make_path_posix(path))
        path = path.rstrip("/") or cls.root_marker
        return drive + path

```
ap--: Thank you @fleming79 for the proposed implementations. I'll try them out shortly, when I find some more time.

I've been collecting more test cases for `_strip_protocol` (still need to do `_parent`). In the test cases I basically try to document current behavior.

So far I noticed that between `v2024.3.0` and `v2024.3.1` there are the following differences in `LocalFileSystem._strip_protocol` behavior (note, that a lot of the test cases are probably just uncommon edge-cases):

**on posix**

| input        | v2024.3.0          | v2024.3.1           |
|--------------|--------------------|---------------------|
| `file://`    | `/current_workdir` | `/current_workdir/` |
| `file://./`  | `/current_workdir` | `/current_workdir/` |
| `./`         | `/current_workdir` | `/current_workdir/` |

**on windows**

| input                        | v2024.3.0             | v2024.3.1                              |
|------------------------------|-----------------------|----------------------------------------|
| `file:/foo/bar`              | `c:/foo/bar`          | `c:/Users/user_home/foo/bar`           |
| `local:/path`                | `c:/path`             | `c:/Users/user_home/path`              |
| `s3://bucket/key`            | `s3://bucket/key`     | `c:/Users/user_home/s3:/bucket/key`    |
| `file:///`                   | `c:/`                 | `c:/Users/user_home/`                  |
| `file:////remote/share/path` | `//remote/share/path` | `c:/Users/user_home/remote/share/path` |


I will write more tests for `_parent`.

(Update: the failing windows test, succeeds locally on a windows py3.11. I'll investigate a bit further...)
ap--: Performance comparison on windows python==3.11, shows that the `2023.3.1` implementation is the fastest and the implementation in this PR is significantly worse.

I'll update this benchmark once I test the implementation provided in the comment above.

```
(venv) PS C:\User\user\development\filesystem_spec> asv run
· Creating environments.........
· Discovering benchmarks
·· Uninstalling from virtualenv-py3.11
·· Building 4bd16f64 <2024.3.0^0> for virtualenv-py3.11...........
·· Installing 4bd16f64 <2024.3.0^0> into virtualenv-py3.11.
· Running 6 total benchmarks (3 commits * 1 environments * 2 benchmarks)
[ 0.00%] · For filesystem_spec commit 4bd16f64 <2024.3.0^0>:
[ 0.00%] ·· Benchmarking virtualenv-py3.11
[ 8.33%] ··· Running (bench_strip_protocol.Suite.time_parent--)..
[25.00%] ··· bench_strip_protocol.Suite.time_parent                                   22.5±0.3ms
[33.33%] ··· bench_strip_protocol.Suite.time_split_protocol                           20.3±0.2ms
[33.33%] · For filesystem_spec commit 516b0626 <local-fs-restore-strip-protocol>:
[33.33%] ·· Building for virtualenv-py3.11........
[33.33%] ·· Benchmarking virtualenv-py3.11
[41.67%] ··· Running (bench_strip_protocol.Suite.time_parent--)..
[58.33%] ··· bench_strip_protocol.Suite.time_parent                                   43.3±0.3ms
[66.67%] ··· bench_strip_protocol.Suite.time_split_protocol                           25.7±0.3ms
[66.67%] · For filesystem_spec commit 47b445ae <2024.3.1^0>:
[66.67%] ·· Building for virtualenv-py3.11...........
[66.67%] ·· Benchmarking virtualenv-py3.11
[75.00%] ··· Running (bench_strip_protocol.Suite.time_parent--)..
[91.67%] ··· bench_strip_protocol.Suite.time_parent                                   19.5±0.3ms
[100.00%] ··· bench_strip_protocol.Suite.time_split_protocol                          16.5±0.2ms
```

Patch:
diff --git a/ci/environment-py38.yml b/ci/environment-py38.yml
index 25f68e5..9c514de 100644
--- a/ci/environment-py38.yml
+++ b/ci/environment-py38.yml
@@ -3,7 +3,7 @@ channels:
   - conda-forge
 dependencies:
   - pip
-  - git
+  - git <2.45.0
   - py
   - pip:
     - hadoop-test-cluster
diff --git a/fsspec/generic.py b/fsspec/generic.py
index 4fb58ac..5e1412c 100644
--- a/fsspec/generic.py
+++ b/fsspec/generic.py
@@ -139,7 +139,7 @@ def rsync(
         source_files, target_files = zip(*allfiles.items())
         fs.cp(source_files, target_files, **kwargs)
     logger.debug(f"{len(to_delete)} files to delete")
-    if delete_missing:
+    if delete_missing and to_delete:
         fs.rm(to_delete)
     return allfiles
 
diff --git a/fsspec/gui.py b/fsspec/gui.py
index 14fc493..113317e 100644
--- a/fsspec/gui.py
+++ b/fsspec/gui.py
@@ -94,7 +94,7 @@ class SigSlot:
         try:
             return self.panel._repr_mimebundle_(*args, **kwargs)
         except (ValueError, AttributeError):
-            raise NotImplementedError("Panel does not seem to be set " "up properly")
+            raise NotImplementedError("Panel does not seem to be set up properly")
 
     def connect(self, signal, slot):
         """Associate call back with given event
diff --git a/fsspec/implementations/local.py b/fsspec/implementations/local.py
index 7938a91..9881606 100644
--- a/fsspec/implementations/local.py
+++ b/fsspec/implementations/local.py
@@ -15,12 +15,6 @@ from fsspec.utils import isfilelike, stringify_path
 logger = logging.getLogger("fsspec.local")
 
 
-def _remove_prefix(text: str, prefix: str):
-    if text.startswith(prefix):
-        return text[len(prefix) :]
-    return text
-
-
 class LocalFileSystem(AbstractFileSystem):
     """Interface to files on local storage
 
@@ -121,8 +115,8 @@ class LocalFileSystem(AbstractFileSystem):
         return osp.lexists(path)
 
     def cp_file(self, path1, path2, **kwargs):
-        path1 = self._strip_protocol(path1, remove_trailing_slash=True)
-        path2 = self._strip_protocol(path2, remove_trailing_slash=True)
+        path1 = self._strip_protocol(path1)
+        path2 = self._strip_protocol(path2)
         if self.auto_mkdir:
             self.makedirs(self._parent(path2), exist_ok=True)
         if self.isfile(path1):
@@ -151,8 +145,8 @@ class LocalFileSystem(AbstractFileSystem):
         return self.cp_file(path1, path2, **kwargs)
 
     def mv(self, path1, path2, **kwargs):
-        path1 = self._strip_protocol(path1, remove_trailing_slash=True)
-        path2 = self._strip_protocol(path2, remove_trailing_slash=True)
+        path1 = self._strip_protocol(path1)
+        path2 = self._strip_protocol(path2)
         shutil.move(path1, path2)
 
     def link(self, src, dst, **kwargs):
@@ -176,7 +170,7 @@ class LocalFileSystem(AbstractFileSystem):
             path = [path]
 
         for p in path:
-            p = self._strip_protocol(p, remove_trailing_slash=True)
+            p = self._strip_protocol(p)
             if self.isdir(p):
                 if not recursive:
                     raise ValueError("Cannot delete directory, set recursive=True")
@@ -219,7 +213,7 @@ class LocalFileSystem(AbstractFileSystem):
 
     @classmethod
     def _parent(cls, path):
-        path = cls._strip_protocol(path, remove_trailing_slash=True)
+        path = cls._strip_protocol(path)
         if os.sep == "/":
             # posix native
             return path.rsplit("/", 1)[0] or "/"
@@ -234,17 +228,43 @@ class LocalFileSystem(AbstractFileSystem):
             return path_
 
     @classmethod
-    def _strip_protocol(cls, path, remove_trailing_slash=False):
+    def _strip_protocol(cls, path):
         path = stringify_path(path)
-        if path.startswith("file:"):
-            path = _remove_prefix(_remove_prefix(path, "file://"), "file:")
-            if os.sep == "\\":
-                path = path.lstrip("/")
+        if path.startswith("file://"):
+            path = path[7:]
+        elif path.startswith("file:"):
+            path = path[5:]
+        elif path.startswith("local://"):
+            path = path[8:]
         elif path.startswith("local:"):
-            path = _remove_prefix(_remove_prefix(path, "local://"), "local:")
-            if os.sep == "\\":
-                path = path.lstrip("/")
-        return make_path_posix(path, remove_trailing_slash)
+            path = path[6:]
+
+        path = make_path_posix(path)
+        if os.sep != "/":
+            # This code-path is a stripped down version of
+            # > drive, path = ntpath.splitdrive(path)
+            if path[1:2] == ":":
+                # Absolute drive-letter path, e.g. X:\Windows
+                # Relative path with drive, e.g. X:Windows
+                drive, path = path[:2], path[2:]
+            elif path[:2] == "//":
+                # UNC drives, e.g. \\server\share or \\?\UNC\server\share
+                # Device drives, e.g. \\.\device or \\?\device
+                if (index1 := path.find("/", 2)) == -1 or (
+                    index2 := path.find("/", index1 + 1)
+                ) == -1:
+                    drive, path = path, ""
+                else:
+                    drive, path = path[:index2], path[index2:]
+            else:
+                # Relative path, e.g. Windows
+                drive = ""
+
+            path = path.rstrip("/") or cls.root_marker
+            return drive + path
+
+        else:
+            return path.rstrip("/") or cls.root_marker
 
     def _isfilestore(self):
         # Inheriting from DaskFileSystem makes this False (S3, etc. were)
@@ -257,42 +277,55 @@ class LocalFileSystem(AbstractFileSystem):
         return os.chmod(path, mode)
 
 
-def make_path_posix(path, remove_trailing_slash=False):
-    """Make path generic for current OS"""
+def make_path_posix(path):
+    """Make path generic and absolute for current OS"""
     if not isinstance(path, str):
         if isinstance(path, (list, set, tuple)):
-            return type(path)(make_path_posix(p, remove_trailing_slash) for p in path)
+            return type(path)(make_path_posix(p) for p in path)
         else:
-            path = str(stringify_path(path))
+            path = stringify_path(path)
+            if not isinstance(path, str):
+                raise TypeError(f"could not convert {path!r} to string")
     if os.sep == "/":
         # Native posix
         if path.startswith("/"):
             # most common fast case for posix
-            return path.rstrip("/") or "/" if remove_trailing_slash else path
+            return path
         elif path.startswith("~"):
-            return make_path_posix(osp.expanduser(path), remove_trailing_slash)
+            return osp.expanduser(path)
         elif path.startswith("./"):
             path = path[2:]
-            path = f"{os.getcwd()}/{path}"
-            return path.rstrip("/") or "/" if remove_trailing_slash else path
+        elif path == ".":
+            path = ""
         return f"{os.getcwd()}/{path}"
     else:
         # NT handling
-        if len(path) > 1:
-            if path[1] == ":":
-                # windows full path like "C:\\local\\path"
-                if len(path) <= 3:
-                    # nt root (something like c:/)
-                    return path[0] + ":/"
-                path = path.replace("\\", "/").replace("//", "/")
-                return path.rstrip("/") if remove_trailing_slash else path
-            elif path[0] == "~":
-                return make_path_posix(osp.expanduser(path), remove_trailing_slash)
-            elif path.startswith(("\\\\", "//")):
-                # windows UNC/DFS-style paths
-                path = "//" + path[2:].replace("\\", "/").replace("//", "/")
-                return path.rstrip("/") if remove_trailing_slash else path
-        return make_path_posix(osp.abspath(path), remove_trailing_slash)
+        if path[0:1] == "/" and path[2:3] == ":":
+            # path is like "/c:/local/path"
+            path = path[1:]
+        if path[1:2] == ":":
+            # windows full path like "C:\\local\\path"
+            if len(path) <= 3:
+                # nt root (something like c:/)
+                return path[0] + ":/"
+            path = path.replace("\\", "/")
+            return path
+        elif path[0:1] == "~":
+            return make_path_posix(osp.expanduser(path))
+        elif path.startswith(("\\\\", "//")):
+            # windows UNC/DFS-style paths
+            return "//" + path[2:].replace("\\", "/")
+        elif path.startswith(("\\", "/")):
+            # windows relative path with root
+            path = path.replace("\\", "/")
+            return f"{osp.splitdrive(os.getcwd())[0]}{path}"
+        else:
+            path = path.replace("\\", "/")
+            if path.startswith("./"):
+                path = path[2:]
+            elif path == ".":
+                path = ""
+            return f"{make_path_posix(os.getcwd())}/{path}"
 
 
 def trailing_sep(path):
diff --git a/fsspec/implementations/smb.py b/fsspec/implementations/smb.py
index 3794993..867a581 100644
--- a/fsspec/implementations/smb.py
+++ b/fsspec/implementations/smb.py
@@ -67,6 +67,7 @@ class SMBFileSystem(AbstractFileSystem):
         timeout=60,
         encrypt=None,
         share_access=None,
+        register_session_retries=5,
         **kwargs,
     ):
         """
@@ -111,6 +112,7 @@ class SMBFileSystem(AbstractFileSystem):
         self.encrypt = encrypt
         self.temppath = kwargs.pop("temppath", "")
         self.share_access = share_access
+        self.register_session_retries = register_session_retries
         self._connect()
 
     @property
@@ -120,7 +122,7 @@ class SMBFileSystem(AbstractFileSystem):
     def _connect(self):
         import time
 
-        for _ in range(5):
+        for _ in range(self.register_session_retries):
             try:
                 smbclient.register_session(
                     self.host,
diff --git a/fsspec/spec.py b/fsspec/spec.py
index 9a7e4e8..bcc01f5 100644
--- a/fsspec/spec.py
+++ b/fsspec/spec.py
@@ -1698,7 +1698,12 @@ class AbstractBufferedFile(io.IOBase):
         """Files are equal if they have the same checksum, only in read mode"""
         if self is other:
             return True
-        return self.mode == "rb" and other.mode == "rb" and hash(self) == hash(other)
+        return (
+            isinstance(other, type(self))
+            and self.mode == "rb"
+            and other.mode == "rb"
+            and hash(self) == hash(other)
+        )
 
     def commit(self):
         """Move from temp to final destination"""
diff --git a/fsspec/utils.py b/fsspec/utils.py
index ba3f80b..6e130aa 100644
--- a/fsspec/utils.py
+++ b/fsspec/utils.py
@@ -350,8 +350,6 @@ def stringify_path(filepath: str | os.PathLike[str] | pathlib.Path) -> str:
         return filepath
     elif hasattr(filepath, "__fspath__"):
         return filepath.__fspath__()
-    elif isinstance(filepath, pathlib.Path):
-        return str(filepath)
     elif hasattr(filepath, "path"):
         return filepath.path
     else:


Test patch:
diff --git a/fsspec/implementations/tests/test_local.py b/fsspec/implementations/tests/test_local.py
index b594cbc..ef39279 100644
--- a/fsspec/implementations/tests/test_local.py
+++ b/fsspec/implementations/tests/test_local.py
@@ -34,12 +34,41 @@ files = {
 }
 
 csv_files = {
-    ".test.fakedata.1.csv": (b"a,b\n" b"1,2\n"),
-    ".test.fakedata.2.csv": (b"a,b\n" b"3,4\n"),
+    ".test.fakedata.1.csv": (b"a,b\n1,2\n"),
+    ".test.fakedata.2.csv": (b"a,b\n3,4\n"),
 }
 odir = os.getcwd()
 
 
+@pytest.fixture()
+def cwd():
+    pth = os.getcwd().replace("\\", "/")
+    assert not pth.endswith("/")
+    yield pth
+
+
+@pytest.fixture()
+def current_drive(cwd):
+    drive = os.path.splitdrive(cwd)[0]
+    assert not drive or (len(drive) == 2 and drive.endswith(":"))
+    yield drive
+
+
+@pytest.fixture()
+def user_home():
+    pth = os.path.expanduser("~").replace("\\", "/")
+    assert not pth.endswith("/")
+    yield pth
+
+
+def winonly(*args):
+    return pytest.param(*args, marks=pytest.mark.skipif(not WIN, reason="Windows only"))
+
+
+def posixonly(*args):
+    return pytest.param(*args, marks=pytest.mark.skipif(WIN, reason="Posix only"))
+
+
 @contextmanager
 def filetexts(d, open=open, mode="t"):
     """Dumps a number of textfiles to disk
@@ -123,21 +152,19 @@ def test_urlpath_expand_read():
 def test_cats():
     with filetexts(csv_files, mode="b"):
         fs = fsspec.filesystem("file")
-        assert fs.cat(".test.fakedata.1.csv") == b"a,b\n" b"1,2\n"
+        assert fs.cat(".test.fakedata.1.csv") == b"a,b\n1,2\n"
         out = set(fs.cat([".test.fakedata.1.csv", ".test.fakedata.2.csv"]).values())
-        assert out == {b"a,b\n" b"1,2\n", b"a,b\n" b"3,4\n"}
-        assert fs.cat(".test.fakedata.1.csv", None, None) == b"a,b\n" b"1,2\n"
-        assert fs.cat(".test.fakedata.1.csv", start=1, end=6) == b"a,b\n" b"1,2\n"[1:6]
-        assert fs.cat(".test.fakedata.1.csv", start=-1) == b"a,b\n" b"1,2\n"[-1:]
-        assert (
-            fs.cat(".test.fakedata.1.csv", start=1, end=-2) == b"a,b\n" b"1,2\n"[1:-2]
-        )
+        assert out == {b"a,b\n1,2\n", b"a,b\n3,4\n"}
+        assert fs.cat(".test.fakedata.1.csv", None, None) == b"a,b\n1,2\n"
+        assert fs.cat(".test.fakedata.1.csv", start=1, end=6) == b"a,b\n1,2\n"[1:6]
+        assert fs.cat(".test.fakedata.1.csv", start=-1) == b"a,b\n1,2\n"[-1:]
+        assert fs.cat(".test.fakedata.1.csv", start=1, end=-2) == b"a,b\n1,2\n"[1:-2]
         out = set(
             fs.cat(
                 [".test.fakedata.1.csv", ".test.fakedata.2.csv"], start=1, end=-1
             ).values()
         )
-        assert out == {b"a,b\n" b"1,2\n"[1:-1], b"a,b\n" b"3,4\n"[1:-1]}
+        assert out == {b"a,b\n1,2\n"[1:-1], b"a,b\n3,4\n"[1:-1]}
 
 
 def test_urlpath_expand_write():
@@ -474,7 +501,6 @@ def test_make_path_posix():
         assert make_path_posix("/posix") == f"{drive}:/posix"
         # Windows drive requires trailing slash
         assert make_path_posix("C:\\") == "C:/"
-        assert make_path_posix("C:\\", remove_trailing_slash=True) == "C:/"
     else:
         assert make_path_posix("/a/posix/path") == "/a/posix/path"
         assert make_path_posix("/posix") == "/posix"
@@ -507,6 +533,71 @@ def test_make_path_posix():
     assert userpath.endswith("/path")
 
 
+@pytest.mark.parametrize(
+    "path",
+    [
+        "/abc/def",
+        "abc/def",
+        "",
+        ".",
+        "//server/share/",
+        "\\\\server\\share\\",
+        "C:\\",
+        "d:/abc/def",
+        "e:",
+        pytest.param(
+            "\\\\server\\share",
+            marks=[
+                pytest.mark.xfail(
+                    WIN and sys.version_info < (3, 11),
+                    reason="requires py3.11+ see: python/cpython#96290",
+                )
+            ],
+        ),
+        pytest.param(
+            "f:foo",
+            marks=[pytest.mark.xfail(WIN, reason="unsupported")],
+            id="relative-path-with-drive",
+        ),
+    ],
+)
+def test_make_path_posix_returns_absolute_paths(path):
+    posix_pth = make_path_posix(path)
+    assert os.path.isabs(posix_pth)
+
+
+@pytest.mark.parametrize("container_cls", [list, set, tuple])
+def test_make_path_posix_set_list_tuple(container_cls):
+    paths = container_cls(
+        [
+            "/foo/bar",
+            "bar/foo",
+        ]
+    )
+    posix_paths = make_path_posix(paths)
+    assert isinstance(posix_paths, container_cls)
+    assert posix_paths == container_cls(
+        [
+            make_path_posix("/foo/bar"),
+            make_path_posix("bar/foo"),
+        ]
+    )
+
+
+@pytest.mark.parametrize(
+    "obj",
+    [
+        1,
+        True,
+        None,
+        object(),
+    ],
+)
+def test_make_path_posix_wrong_type(obj):
+    with pytest.raises(TypeError):
+        make_path_posix(obj)
+
+
 def test_parent():
     if WIN:
         assert LocalFileSystem._parent("C:\\file or folder") == "C:/"
@@ -516,6 +607,48 @@ def test_parent():
         assert LocalFileSystem._parent("/") == "/"
 
 
+@pytest.mark.parametrize(
+    "path,parent",
+    [
+        ("C:\\", "C:/"),
+        ("C:\\.", "C:/"),
+        ("C:\\.\\", "C:/"),
+        ("file:C:/", "C:/"),
+        ("file://C:/", "C:/"),
+        ("local:C:/", "C:/"),
+        ("local://C:/", "C:/"),
+        ("\\\\server\\share", "//server/share"),
+        ("\\\\server\\share\\", "//server/share"),
+        ("\\\\server\\share\\path", "//server/share"),
+        ("//server/share", "//server/share"),
+        ("//server/share/", "//server/share"),
+        ("//server/share/path", "//server/share"),
+        ("C:\\file or folder", "C:/"),
+        ("C:\\file or folder\\", "C:/"),
+        ("file:///", "{current_drive}/"),
+        ("file:///path", "{current_drive}/"),
+    ]
+    if WIN
+    else [
+        ("/", "/"),
+        ("/.", "/"),
+        ("/./", "/"),
+        ("file:/", "/"),
+        ("file:///", "/"),
+        ("local:/", "/"),
+        ("local:///", "/"),
+        ("/file or folder", "/"),
+        ("/file or folder/", "/"),
+        ("file:///path", "/"),
+        ("file://c/", "{cwd}"),
+    ],
+)
+def test_parent_edge_cases(path, parent, cwd, current_drive):
+    parent = parent.format(cwd=cwd, current_drive=current_drive)
+
+    assert LocalFileSystem._parent(path) == parent
+
+
 def test_linked_files(tmpdir):
     tmpdir = str(tmpdir)
     fn0 = os.path.join(tmpdir, "target")
@@ -649,26 +782,130 @@ def test_pickle(tmpdir):
         assert f2.read() == f.read()
 
 
-def test_strip_protocol_expanduser():
-    path = "file://~\\foo\\bar" if WIN else "file://~/foo/bar"
-    stripped = LocalFileSystem._strip_protocol(path)
-    assert path != stripped
-    assert "~" not in stripped
-    assert "file://" not in stripped
-    assert stripped.startswith(os.path.expanduser("~").replace("\\", "/"))
-    path = LocalFileSystem._strip_protocol("./", remove_trailing_slash=True)
-    assert not path.endswith("/")
+@pytest.mark.parametrize(
+    "uri, expected",
+    [
+        ("file://~/foo/bar", "{user_home}/foo/bar"),
+        ("~/foo/bar", "{user_home}/foo/bar"),
+        winonly("~\\foo\\bar", "{user_home}/foo/bar"),
+        winonly("file://~\\foo\\bar", "{user_home}/foo/bar"),
+    ],
+)
+def test_strip_protocol_expanduser(uri, expected, user_home):
+    expected = expected.format(user_home=user_home)
 
+    stripped = LocalFileSystem._strip_protocol(uri)
+    assert expected == stripped
 
-def test_strip_protocol_no_authority():
-    path = "file:\\foo\\bar" if WIN else "file:/foo/bar"
-    stripped = LocalFileSystem._strip_protocol(path)
-    assert "file:" not in stripped
-    assert stripped.endswith("/foo/bar")
-    if WIN:
-        assert (
-            LocalFileSystem._strip_protocol("file://C:\\path\\file") == "C:/path/file"
-        )
+
+@pytest.mark.parametrize(
+    "uri, expected",
+    [
+        ("file://", "{cwd}"),
+        ("file://.", "{cwd}"),
+        ("file://./", "{cwd}"),
+        ("./", "{cwd}"),
+        ("file:path", "{cwd}/path"),
+        ("file://path", "{cwd}/path"),
+        ("path", "{cwd}/path"),
+        ("./path", "{cwd}/path"),
+        winonly(".\\", "{cwd}"),
+        winonly("file://.\\path", "{cwd}/path"),
+    ],
+)
+def test_strip_protocol_relative_paths(uri, expected, cwd):
+    expected = expected.format(cwd=cwd)
+
+    stripped = LocalFileSystem._strip_protocol(uri)
+    assert expected == stripped
+
+
+@pytest.mark.parametrize(
+    "uri, expected",
+    [
+        posixonly("file:/foo/bar", "/foo/bar"),
+        winonly("file:/foo/bar", "{current_drive}/foo/bar"),
+        winonly("file:\\foo\\bar", "{current_drive}/foo/bar"),
+        winonly("file:D:\\path\\file", "D:/path/file"),
+        winonly("file:/D:\\path\\file", "D:/path/file"),
+        winonly("file://D:\\path\\file", "D:/path/file"),
+    ],
+)
+def test_strip_protocol_no_authority(uri, expected, cwd, current_drive):
+    expected = expected.format(cwd=cwd, current_drive=current_drive)
+
+    stripped = LocalFileSystem._strip_protocol(uri)
+    assert expected == stripped
+
+
+@pytest.mark.parametrize(
+    "uri, expected",
+    [
+        ("file:/path", "/path"),
+        ("file:///path", "/path"),
+        ("file:////path", "//path"),
+        ("local:/path", "/path"),
+        ("s3://bucket/key", "{cwd}/s3://bucket/key"),
+        ("/path", "/path"),
+        ("file:///", "/"),
+    ]
+    if not WIN
+    else [
+        ("file:c:/path", "c:/path"),
+        ("file:/c:/path", "c:/path"),
+        ("file:/C:/path", "C:/path"),
+        ("file://c:/path", "c:/path"),
+        ("file:///c:/path", "c:/path"),
+        ("local:/path", "{current_drive}/path"),
+        ("s3://bucket/key", "{cwd}/s3://bucket/key"),
+        ("c:/path", "c:/path"),
+        ("c:\\path", "c:/path"),
+        ("file:///", "{current_drive}/"),
+        pytest.param(
+            "file://localhost/c:/path",
+            "c:/path",
+            marks=pytest.mark.xfail(
+                reason="rfc8089 section3 'localhost uri' not supported"
+            ),
+        ),
+    ],
+)
+def test_strip_protocol_absolute_paths(uri, expected, current_drive, cwd):
+    expected = expected.format(current_drive=current_drive, cwd=cwd)
+
+    stripped = LocalFileSystem._strip_protocol(uri)
+    assert expected == stripped
+
+
+@pytest.mark.parametrize(
+    "uri, expected",
+    [
+        ("file:c|/path", "c:/path"),
+        ("file:/D|/path", "D:/path"),
+        ("file:///C|/path", "C:/path"),
+    ],
+)
+@pytest.mark.skipif(not WIN, reason="Windows only")
+@pytest.mark.xfail(WIN, reason="legacy dos uris not supported")
+def test_strip_protocol_legacy_dos_uris(uri, expected):
+    stripped = LocalFileSystem._strip_protocol(uri)
+    assert expected == stripped
+
+
+@pytest.mark.parametrize(
+    "uri, stripped",
+    [
+        ("file://remote/share/pth", "{cwd}/remote/share/pth"),
+        ("file:////remote/share/pth", "//remote/share/pth"),
+        ("file://///remote/share/pth", "///remote/share/pth"),
+        ("//remote/share/pth", "//remote/share/pth"),
+        winonly("\\\\remote\\share\\pth", "//remote/share/pth"),
+    ],
+)
+def test_strip_protocol_windows_remote_shares(uri, stripped, cwd):
+    stripped = stripped.format(cwd=cwd)
+
+    assert LocalFileSystem._strip_protocol(uri) == stripped
 
 
 def test_mkdir_twice_faile(tmpdir):
diff --git a/fsspec/implementations/tests/test_sftp.py b/fsspec/implementations/tests/test_sftp.py
index 1381fda..b91f0b5 100644
--- a/fsspec/implementations/tests/test_sftp.py
+++ b/fsspec/implementations/tests/test_sftp.py
@@ -114,7 +114,7 @@ def test_get_dir(protocol, ssh, root_path, tmpdir):
 
     f.get(
         protocol
-        + "://{username}:{password}@{host}:{port}" "{root_path}".format(
+        + "://{username}:{password}@{host}:{port}{root_path}".format(
             root_path=root_path, **ssh
         ),
         f"{path}/test2",
diff --git a/fsspec/implementations/tests/test_smb.py b/fsspec/implementations/tests/test_smb.py
index da65127..9b2afcd 100644
--- a/fsspec/implementations/tests/test_smb.py
+++ b/fsspec/implementations/tests/test_smb.py
@@ -63,6 +63,7 @@ def smb_params(request):
             "port": request.param,
             "username": "testuser",
             "password": "testpass",
+            "register_session_retries": 100,  # max ~= 10 seconds
         }
     finally:
         import smbclient  # pylint: disable=import-outside-toplevel
diff --git a/fsspec/tests/test_api.py b/fsspec/tests/test_api.py
index dd3374f..24e6101 100644
--- a/fsspec/tests/test_api.py
+++ b/fsspec/tests/test_api.py
@@ -191,7 +191,7 @@ def test_pipe_cat(m):
 def test_read_block_delimiter(m):
     fs = MemoryFileSystem()
     with fs.open("/myfile", "wb") as f:
-        f.write(b"some\n" b"lines\n" b"of\n" b"text")
+        f.write(b"some\nlines\nof\ntext")
     assert fs.read_block("/myfile", 0, 2, b"\n") == b"some\n"
     assert fs.read_block("/myfile", 2, 6, b"\n") == b"lines\n"
     assert fs.read_block("/myfile", 6, 2, b"\n") == b""
@@ -203,7 +203,7 @@ def test_read_block_delimiter(m):
 def test_open_text(m):
     fs = MemoryFileSystem()
     with fs.open("/myfile", "wb") as f:
-        f.write(b"some\n" b"lines\n" b"of\n" b"text")
+        f.write(b"some\nlines\nof\ntext")
     f = fs.open("/myfile", "r", encoding="latin1")
     assert f.encoding == "latin1"
 
diff --git a/fsspec/tests/test_spec.py b/fsspec/tests/test_spec.py
index 31788d8..de23d78 100644
--- a/fsspec/tests/test_spec.py
+++ b/fsspec/tests/test_spec.py
@@ -819,6 +819,10 @@ def test_eq():
     result = fs == 1
     assert result is False
 
+    f = AbstractBufferedFile(fs, "misc/foo.txt", cache_type="bytes")
+    result = f == 1
+    assert result is False
+
 
 def test_pickle_multiple():
     a = DummyTestFS(1)
diff --git a/fsspec/tests/test_utils.py b/fsspec/tests/test_utils.py
index 0efd5d9..10fa89a 100644
--- a/fsspec/tests/test_utils.py
+++ b/fsspec/tests/test_utils.py
@@ -1,6 +1,6 @@
 import io
 import sys
-from pathlib import Path
+from pathlib import Path, PurePath
 from unittest.mock import Mock
 
 import pytest
@@ -440,3 +440,39 @@ def test_size():
     f = io.BytesIO(b"hello")
     assert fsspec.utils.file_size(f) == 5
     assert f.tell() == 0
+
+
+class _HasFspath:
+    def __fspath__(self):
+        return "foo"
+
+
+class _HasPathAttr:
+    def __init__(self):
+        self.path = "foo"
+
+
+@pytest.mark.parametrize(
+    "path,expected",
+    [
+        # coerce to string
+        ("foo", "foo"),
+        (Path("foo"), "foo"),
+        (PurePath("foo"), "foo"),
+        (_HasFspath(), "foo"),
+        (_HasPathAttr(), "foo"),
+        # passthrough
+        (b"bytes", b"bytes"),
+        (None, None),
+        (1, 1),
+        (True, True),
+        (o := object(), o),
+        ([], []),
+        ((), ()),
+        (set(), set()),
+    ],
+)
+def test_stringify_path(path, expected):
+    path = fsspec.utils.stringify_path(path)
+
+    assert path == expected