RDFLib / rdflib

RDFLib is a Python library for working with RDF, a simple yet powerful language for representing information.
https://rdflib.readthedocs.org
BSD 3-Clause "New" or "Revised" License
2.11k stars 547 forks source link

TestUtil.test_to_bits_from_bits_round_trip test failure with python3.13b2 #2801

Open mtasaka opened 2 weeks ago

mtasaka commented 2 weeks ago

Fedora project is now planning to import python 3.13 into Fedora 40, and now trying to rebuild all packages in Fedora with python 3.13b2 .

With rdflib 7.0.0, testsuite TestUtil.test_to_bits_from_bits_round_trip fails with python 3.13b2 as:

__________________ TestUtil.test_to_bits_from_bits_round_trip __________________
self = <test_nodepickler.TestUtil object at 0x7fe577cee490>
        def test_to_bits_from_bits_round_trip(self):
            np = NodePickler()

            a = Literal(
                """A test with a \\n (backslash n), "\u00a9" , and newline \n and a second line.
    """
            )
>           b = np.loads(np.dumps(a))
test/test_store/test_nodepickler.py:32: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <rdflib.store.NodePickler object at 0x7fe5742e5010>
obj = rdflib.term.Literal('A test with a \\n (backslash n), "©" , and newline \n and a second line.\n')
protocol = None, bin = None
    def dumps(
        self, obj: "Node", protocol: Optional[Any] = None, bin: Optional[Any] = None
    ):
        src = BytesIO()
        p = Pickler(src)
        # NOTE on type error: https://github.com/python/mypy/issues/2427
        # type error: Cannot assign to a method
>       p.persistent_id = self._get_ids  # type: ignore[assignment]
E       AttributeError: '_pickle.Pickler' object attribute 'persistent_id' is read-only
rdflib/store.py:149: AttributeError
_________________________ TestUtil.test_literal_cases __________________________
self = <test_nodepickler.TestUtil object at 0x7fe577cee5d0>
    def test_literal_cases(self):
        np = NodePickler()

        for l in cases:
            a = Literal(l)
>           b = np.loads(np.dumps(a))
test/test_store/test_nodepickler.py:40: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <rdflib.store.NodePickler object at 0x7fe5742cd450>
obj = rdflib.term.Literal('no quotes'), protocol = None, bin = None
    def dumps(
        self, obj: "Node", protocol: Optional[Any] = None, bin: Optional[Any] = None
    ):
        src = BytesIO()
        p = Pickler(src)
        # NOTE on type error: https://github.com/python/mypy/issues/2427
        # type error: Cannot assign to a method
>       p.persistent_id = self._get_ids  # type: ignore[assignment]
E       AttributeError: '_pickle.Pickler' object attribute 'persistent_id' is read-only
rdflib/store.py:149: AttributeError

Looks like this assignment is no longer accepted.

https://github.com/RDFLib/rdflib/blob/fcfc2aa90c25ee950cf0b9994cb00524c09d98f3/rdflib/store.py#L136 https://github.com/RDFLib/rdflib/blob/fcfc2aa90c25ee950cf0b9994cb00524c09d98f3/rdflib/store.py#L149

mtasaka commented 2 weeks ago

I don't know python well, however looking at test code in python source (itself), the following seems to work (again, I am not python expert)

--- rdflib-7.0.0/rdflib/store.py.orig   2023-08-02 02:57:32.691258000 +0900
+++ rdflib-7.0.0/rdflib/store.py    2024-06-13 14:15:59.682909998 +0900
@@ -130,10 +130,11 @@ class NodePickler:
         self._ids[object] = id

     def loads(self, s: bytes) -> "Node":
-        up = Unpickler(BytesIO(s))
-        # NOTE on type error: https://github.com/python/mypy/issues/2427
-        # type error: Cannot assign to a method
-        up.persistent_load = self._get_object  # type: ignore[assignment]
+        class LoadsUnpickler(Unpickler):
+            def persistent_load(subself, obj):
+                return self._get_object(obj)
+
+        up = LoadsUnpickler(BytesIO(s))
         try:
             return up.load()
         except KeyError as e:
@@ -142,11 +143,12 @@ class NodePickler:
     def dumps(
         self, obj: "Node", protocol: Optional[Any] = None, bin: Optional[Any] = None
     ):
+        class DumpsPickler(Pickler):
+            def persistent_id(subself, key):
+                return self._get_ids(key)
+
         src = BytesIO()
-        p = Pickler(src)
-        # NOTE on type error: https://github.com/python/mypy/issues/2427
-        # type error: Cannot assign to a method
-        p.persistent_id = self._get_ids  # type: ignore[assignment]
+        p = DumpsPickler(src)
         p.dump(obj)
         return src.getvalue()
eclipseo commented 2 weeks ago

Ref: https://github.com/python/cpython/issues/89988 https://github.com/python/cpython/commit/89cee94b315c88d3cd4c9ffc051e7abd6a5f2196

michel-slm commented 2 weeks ago

I did something similar in https://src.fedoraproject.org/rpms/python-rdflib/pull-request/5#request_diff but @mtasaka 's proposal seems cleaner (it kept the new classes scoped locally so does not change the API)

rathann commented 2 weeks ago

Fedora project is now planning to import python 3.13 into Fedora 40,

You mean Fedora 41.