Repository: fsspec/filesystem_spec
Pull request #1632 (2024-06-22 05:41:59)

Fix dict representation not being JSON serializable
This PR fixes `AbstractFileSystem.to_dict()` failing to serialize recursive filesystem/paths which caused the output to not be JSON serializable.

DarkLight1337: Looks like we might need to lock the NumPy version to avoid binary incompatibility with SciPy.
martindurant: You mean this cryptic message on windows only?
> OSError: Error loading libscipy_openblas64_-fb1711452d4d8cee9f276fd1449ee5c7.dll; The operation completed successfully.

I don't know where the scipy dependency is coming from, but we could remove it in the windows run.
DarkLight1337: > You mean this cryptic message on windows only?
> 
> > OSError: Error loading libscipy_openblas64_-fb1711452d4d8cee9f276fd1449ee5c7.dll; The operation completed successfully.
> 
> I don't know where the scipy dependency is coming from, but we could remove it in the windows run.

Yeah.

Patch:
diff --git a/fsspec/implementations/reference.py b/fsspec/implementations/reference.py
index 6e3de91..981e698 100644
--- a/fsspec/implementations/reference.py
+++ b/fsspec/implementations/reference.py
@@ -935,10 +935,13 @@ class ReferenceFileSystem(AsyncFileSystem):
 
     def _process_references0(self, references):
         """Make reference dict for Spec Version 0"""
-        references = {
-            key: json.dumps(val) if isinstance(val, dict) else val
-            for key, val in references.items()
-        }
+        if isinstance(references, dict):
+            # do not do this for lazy/parquet backend, which will not make dicts,
+            # but must remain writable in the original object
+            references = {
+                key: json.dumps(val) if isinstance(val, dict) else val
+                for key, val in references.items()
+            }
         self.references = references
 
     def _process_references1(self, references, template_overrides=None):
diff --git a/fsspec/json.py b/fsspec/json.py
index 81d1f3a..69cead0 100644
--- a/fsspec/json.py
+++ b/fsspec/json.py
@@ -1,7 +1,17 @@
 import json
 from contextlib import suppress
 from pathlib import PurePath
-from typing import Any, Callable, ClassVar, Dict, List, Optional, Tuple
+from typing import (
+    Any,
+    Callable,
+    ClassVar,
+    Dict,
+    List,
+    Mapping,
+    Optional,
+    Sequence,
+    Tuple,
+)
 
 from .registry import _import_class, get_filesystem_class
 from .spec import AbstractFileSystem
@@ -19,6 +29,21 @@ class FilesystemJSONEncoder(json.JSONEncoder):
 
         return super().default(o)
 
+    def make_serializable(self, obj: Any) -> Any:
+        """
+        Recursively converts an object so that it can be JSON serialized via
+        :func:`json.dumps` and :func:`json.dump`, without actually calling
+        said functions.
+        """
+        if isinstance(obj, (str, int, float, bool)):
+            return obj
+        if isinstance(obj, Mapping):
+            return {k: self.make_serializable(v) for k, v in obj.items()}
+        if isinstance(obj, Sequence):
+            return [self.make_serializable(v) for v in obj]
+
+        return self.default(obj)
+
 
 class FilesystemJSONDecoder(json.JSONDecoder):
     def __init__(
@@ -81,3 +106,16 @@ class FilesystemJSONDecoder(json.JSONDecoder):
             return self.original_object_hook(dct)
 
         return dct
+
+    def unmake_serializable(self, obj: Any) -> Any:
+        """
+        Inverse function of :meth:`FilesystemJSONEncoder.make_serializable`.
+        """
+        if isinstance(obj, dict):
+            obj = self.custom_object_hook(obj)
+        if isinstance(obj, dict):
+            return {k: self.unmake_serializable(v) for k, v in obj.items()}
+        if isinstance(obj, (list, tuple)):
+            return [self.unmake_serializable(v) for v in obj]
+
+        return obj
diff --git a/fsspec/spec.py b/fsspec/spec.py
index edc7abf..1463a44 100644
--- a/fsspec/spec.py
+++ b/fsspec/spec.py
@@ -1466,6 +1466,10 @@ class AbstractFileSystem(metaclass=_Cached):
         passed to the constructor, such as passwords and tokens. Make sure you
         store and send them in a secure environment!
         """
+        from .json import FilesystemJSONEncoder
+
+        json_encoder = FilesystemJSONEncoder()
+
         cls = type(self)
         proto = self.protocol
 
@@ -1476,8 +1480,8 @@ class AbstractFileSystem(metaclass=_Cached):
         return dict(
             cls=f"{cls.__module__}:{cls.__name__}",
             protocol=proto[0] if isinstance(proto, (tuple, list)) else proto,
-            args=self.storage_args,
-            **storage_options,
+            args=json_encoder.make_serializable(self.storage_args),
+            **json_encoder.make_serializable(storage_options),
         )
 
     @staticmethod
@@ -1503,6 +1507,8 @@ class AbstractFileSystem(metaclass=_Cached):
         """
         from .json import FilesystemJSONDecoder
 
+        json_decoder = FilesystemJSONDecoder()
+
         dct = dict(dct)  # Defensive copy
 
         cls = FilesystemJSONDecoder.try_resolve_fs_cls(dct)
@@ -1512,7 +1518,10 @@ class AbstractFileSystem(metaclass=_Cached):
         dct.pop("cls", None)
         dct.pop("protocol", None)
 
-        return cls(*dct.pop("args", ()), **dct)
+        return cls(
+            *json_decoder.unmake_serializable(dct.pop("args", ())),
+            **json_decoder.unmake_serializable(dct),
+        )
 
     def _get_pyarrow_filesystem(self):
         """


Test patch:
diff --git a/fsspec/tests/test_spec.py b/fsspec/tests/test_spec.py
index 2bb948b..e11b7ab 100644
--- a/fsspec/tests/test_spec.py
+++ b/fsspec/tests/test_spec.py
@@ -875,17 +875,38 @@ def test_json_path_attr():
 
 def test_json_fs_attr():
     a = DummyTestFS(1)
-    b = DummyTestFS(2, bar=a)
+    b = DummyTestFS(2, bar=Path("baz"))
+    c = DummyTestFS(3, baz=b)
 
     outa = a.to_json()
     outb = b.to_json()
+    outc = c.to_json()
 
-    assert json.loads(outb)  # is valid JSON
-    assert a != b
-    assert "bar" in outb
+    assert json.loads(outc)  # is valid JSON
+    assert b != c
+    assert "baz" in outc
+
+    assert DummyTestFS.from_json(outa) is a
+    assert DummyTestFS.from_json(outb) is b
+    assert DummyTestFS.from_json(outc) is c
+
+
+def test_json_dict_attr():
+    a = DummyTestFS(1)
+    b = DummyTestFS(2, bar=Path("baz"))
+    c = DummyTestFS(3, baz={"key": b})
+
+    outa = a.to_json()
+    outb = b.to_json()
+    outc = c.to_json()
+
+    assert json.loads(outc)  # is valid JSON
+    assert b != c
+    assert "baz" in outc
 
     assert DummyTestFS.from_json(outa) is a
     assert DummyTestFS.from_json(outb) is b
+    assert DummyTestFS.from_json(outc) is c
 
 
 def test_dict():
@@ -903,6 +924,57 @@ def test_dict():
     assert DummyTestFS.from_dict(outb) is b
 
 
+def test_dict_path_attr():
+    a = DummyTestFS(1)
+    b = DummyTestFS(2, bar=Path("baz"))
+
+    outa = a.to_dict()
+    outb = b.to_dict()
+
+    assert isinstance(outa, dict)
+    assert a != b
+    assert outb["bar"]["str"] == "baz"
+
+    assert DummyTestFS.from_dict(outa) is a
+    assert DummyTestFS.from_dict(outb) is b
+
+
+def test_dict_fs_attr():
+    a = DummyTestFS(1)
+    b = DummyTestFS(2, bar=Path("baz"))
+    c = DummyTestFS(3, baz=b)
+
+    outa = a.to_dict()
+    outb = b.to_dict()
+    outc = c.to_dict()
+
+    assert isinstance(outc, dict)
+    assert b != c
+    assert outc["baz"] == outb
+
+    assert DummyTestFS.from_dict(outa) is a
+    assert DummyTestFS.from_dict(outb) is b
+    assert DummyTestFS.from_dict(outc) is c
+
+
+def test_dict_dict_attr():
+    a = DummyTestFS(1)
+    b = DummyTestFS(2, bar=Path("baz"))
+    c = DummyTestFS(3, baz={"key": b})
+
+    outa = a.to_dict()
+    outb = b.to_dict()
+    outc = c.to_dict()
+
+    assert isinstance(outc, dict)
+    assert b != c
+    assert outc["baz"]["key"] == outb
+
+    assert DummyTestFS.from_dict(outa) is a
+    assert DummyTestFS.from_dict(outb) is b
+    assert DummyTestFS.from_dict(outc) is c
+
+
 def test_dict_idempotent():
     a = DummyTestFS(1)
 
@@ -912,6 +984,20 @@ def test_dict_idempotent():
     assert DummyTestFS.from_dict(outa) is a
 
 
+def test_dict_json_serializable():
+    a = DummyTestFS(1)
+    b = DummyTestFS(2, bar=Path("baz"))
+    c = DummyTestFS(3, baz=b)
+
+    outa = a.to_dict()
+    outb = b.to_dict()
+    outc = c.to_dict()
+
+    json.dumps(outa)
+    json.dumps(outb)
+    json.dumps(outc)
+
+
 def test_serialize_no_password():
     fs = DummyTestFS(1, password="admin")