Clobber default change - follow up (#1249)

* Let register_implementation pass without clobber if same class

* Allow non-clobber register is no change

Author: martindurant

fsspec/registry.py

@@ -34,21 +34,25 @@ def register_implementation(name, cls, clobber=False, errtxt=None):
     """
     if isinstance(cls, str):
         if name in known_implementations and clobber is False:
-            raise ValueError(
-                "Name (%s) already in the known_implementations and clobber "
-                "is False" % name
-            )
-        known_implementations[name] = {
-            "class": cls,
-            "err": errtxt or "%s import failed for protocol %s" % (cls, name),
-        }
+            if cls != known_implementations[name]["class"]:
+                raise ValueError(
+                    "Name (%s) already in the known_implementations and clobber "
+                    "is False" % name
+                )
+        else:
+            known_implementations[name] = {
+                "class": cls,
+                "err": errtxt or "%s import failed for protocol %s" % (cls, name),
+            }
 
     else:
         if name in registry and clobber is False:
-            raise ValueError(
-                "Name (%s) already in the registry and clobber is False" % name
-            )
-        _registry[name] = cls
+            if _registry[name] is not cls:
+                raise ValueError(
+                    "Name (%s) already in the registry and clobber is False" % name
+                )
+        else:
+            _registry[name] = cls
 
 
 # protocols mapped to the class which implements them. This dict can

fsspec/tests/test_registry.py

@@ -4,6 +4,8 @@
 
 import pytest
 
+import fsspec
+from fsspec.implementations.zip import ZipFileSystem
 from fsspec.registry import (
     _registry,
     filesystem,
@@ -69,12 +71,16 @@ def test_register_fail(clear_registry):
     with pytest.raises(ImportError):
         get_filesystem_class("test")
 
+    # NOOP
+    register_implementation("test", "doesntexist.AbstractFileSystem", clobber=False)
     with pytest.raises(ValueError):
-        register_implementation("test", "doesntexist.AbstractFileSystem", clobber=False)
+        register_implementation(
+            "test", "doesntexist.AbstractFileSystemm", clobber=False
+        )
 
     # by default we do not allow clobbering
     with pytest.raises(ValueError):
-        register_implementation("test", "doesntexist.AbstractFileSystem")
+        register_implementation("test", "doesntexist.AbstractFileSystemm")
 
     register_implementation(
         "test", "doesntexist.AbstractFileSystem", errtxt="hiho", clobber=True
@@ -84,9 +90,12 @@ def test_register_fail(clear_registry):
     assert "hiho" in str(e.value)
     register_implementation("test", AbstractFileSystem)
 
+    # NOOP
+    register_implementation("test", AbstractFileSystem)
     with pytest.raises(ValueError):
-        register_implementation("test", AbstractFileSystem)
+        register_implementation("test", ZipFileSystem)
     register_implementation("test", AbstractFileSystem, clobber=True)
+    assert isinstance(fsspec.filesystem("test"), AbstractFileSystem)
 
 
 def test_entry_points_registered_on_import(clear_registry, clean_imports):