fix: formatting of SequencePath and AlternativePath (#2504)

These path types were formatted without parentheses even if they contained multiple elements, resulting in string representations that did not accurately represent the path. This change fixes the formatting so that the string representations are enclosed in parentheses when necessary. - Fixes <#2503>.
RDFLib · Jul 31, 2023 · 9c73581 · 9c73581
1 parent bd797ac
commit 9c73581
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 11 deletions.
diff --git a/rdflib/paths.py b/rdflib/paths.py
@@ -213,6 +213,15 @@
 ZeroOrOne = "?"
 
 
+def _n3(
+    arg: Union["URIRef", "Path"], namespace_manager: Optional["NamespaceManager"] = None
+) -> str:
+    # type error: Item "Path" of "Union[Path, URIRef]" has no attribute "n3"  [union-attr]
+    if isinstance(arg, (SequencePath, AlternativePath)) and len(arg.args) > 1:
+        return "(%s)" % arg.n3(namespace_manager)
+    return arg.n3(namespace_manager)  # type: ignore[union-attr]
+
+
 @total_ordering
 class Path:
     __or__: Callable[["Path", Union["URIRef", "Path"]], "AlternativePath"]
@@ -260,8 +269,7 @@ def __repr__(self) -> str:
         return "Path(~%s)" % (self.arg,)
 
     def n3(self, namespace_manager: Optional["NamespaceManager"] = None) -> str:
-        # type error: Item "Path" of "Union[Path, URIRef]" has no attribute "n3"  [union-attr]
-        return "^%s" % self.arg.n3(namespace_manager)  # type: ignore[union-attr]
+        return "^%s" % _n3(self.arg, namespace_manager)
 
 
 class SequencePath(Path):
@@ -318,8 +326,7 @@ def __repr__(self) -> str:
         return "Path(%s)" % " / ".join(str(x) for x in self.args)
 
     def n3(self, namespace_manager: Optional["NamespaceManager"] = None) -> str:
-        # type error: Item "Path" of "Union[Path, URIRef]" has no attribute "n3"  [union-attr]
-        return "/".join(a.n3(namespace_manager) for a in self.args)  # type: ignore[union-attr]
+        return "/".join(_n3(a, namespace_manager) for a in self.args)
 
 
 class AlternativePath(Path):
@@ -345,8 +352,7 @@ def __repr__(self) -> str:
         return "Path(%s)" % " | ".join(str(x) for x in self.args)
 
     def n3(self, namespace_manager: Optional["NamespaceManager"] = None) -> str:
-        # type error: Item "Path" of "Union[Path, URIRef]" has no attribute "n3"  [union-attr]
-        return "|".join(a.n3(namespace_manager) for a in self.args)  # type: ignore[union-attr]
+        return "|".join(_n3(a, namespace_manager) for a in self.args)
 
 
 class MulPath(Path):
@@ -470,8 +476,7 @@ def __repr__(self) -> str:
         return "Path(%s%s)" % (self.path, self.mod)
 
     def n3(self, namespace_manager: Optional["NamespaceManager"] = None) -> str:
-        # type error: Item "Path" of "Union[Path, URIRef]" has no attribute "n3"  [union-attr]
-        return "%s%s" % (self.path.n3(namespace_manager), self.mod)  # type: ignore[union-attr]
+        return "%s%s" % (_n3(self.path, namespace_manager), self.mod)
 
 
 class NegatedPath(Path):
@@ -505,8 +510,7 @@ def __repr__(self) -> str:
         return "Path(! %s)" % ",".join(str(x) for x in self.args)
 
     def n3(self, namespace_manager: Optional["NamespaceManager"] = None) -> str:
-        # type error: Item "Path" of "Union[Path, URIRef]" has no attribute "n3"  [union-attr]
-        return "!(%s)" % ("|".join(arg.n3(namespace_manager) for arg in self.args))  # type: ignore[union-attr]
+        return "!(%s)" % ("|".join(_n3(arg, namespace_manager) for arg in self.args))
 
 
 class PathList(list):

diff --git a/test/test_path.py b/test/test_path.py
@@ -54,15 +54,40 @@
             "rdfs:subClassOf?",
         ),
         (
-            RDF.type / RDFS.subClassOf * "*",
+            RDF.type / MulPath(RDFS.subClassOf, "*"),
             f"<{RDF.type}>/<{RDFS.subClassOf}>*",
             "rdf:type/rdfs:subClassOf*",
         ),
+        (
+            RDF.type / ((SequencePath(RDFS.subClassOf)) * "*"),
+            f"<{RDF.type}>/<{RDFS.subClassOf}>*",
+            "rdf:type/rdfs:subClassOf*",
+        ),
+        (
+            RDF.type / RDFS.subClassOf * "*",
+            f"(<{RDF.type}>/<{RDFS.subClassOf}>)*",
+            "(rdf:type/rdfs:subClassOf)*",
+        ),
         (
             -(RDF.type | RDFS.subClassOf),
             f"!(<{RDF.type}>|<{RDFS.subClassOf}>)",
             "!(rdf:type|rdfs:subClassOf)",
         ),
+        (
+            -(RDF.type | ((SequencePath(RDFS.subClassOf)) * "*")),
+            f"!(<{RDF.type}>|<{RDFS.subClassOf}>*)",
+            "!(rdf:type|rdfs:subClassOf*)",
+        ),
+        (
+            SequencePath(RDFS.subClassOf),
+            f"<{RDFS.subClassOf}>",
+            "rdfs:subClassOf",
+        ),
+        (
+            AlternativePath(RDFS.subClassOf),
+            f"<{RDFS.subClassOf}>",
+            "rdfs:subClassOf",
+        ),
     ],
 )
 def test_paths_n3(