N3 parser: do not create formulas if the Turtle mode is activated #1142
Conversation
|
Hi @ashleysommer! Thank you for your feedback and sorry for the buggy change. I have added a test to ensure there is no regression. |
| @@ -1745,7 +1746,7 @@ def newBlankNode(self, arg=None, uri=None, why=None): | |||
| return arg.newBlankNode(uri) | |||
| elif isinstance(arg, Graph) or arg is None: | |||
| self.counter += 1 | |||
| bn = BNode("n" + str(self.counter)) | |||
| bn = BNode("n%sb%s" % (self.uuid, self.counter)) | |||
There was a problem hiding this comment.
I'm going to have to look into this a little deeper. I don't know if adding a UUID to the RDFSink is the best way to approach this.
There was a problem hiding this comment.
UUID usage is what is currently done internally by the formula blank node generator
| # with formula | ||
| graph = Graph() | ||
| self.assertTrue(graph.store.formula_aware) | ||
| graph.load(BytesIO(file), format=format) |
There was a problem hiding this comment.
Just FYI, in RDFLib 5.1.0 the graph.load() method will be deprecated as part of the Graph API cleanup process, and in 6.0.0 load() will be removed.
You will need to use graph.parse() instead. In this case where file is a string with the RDF data, use graph.parse(data=file).
There was a problem hiding this comment.
Ok! Thank you! I'll fix it if the global PR design is accepted
|
@Tpt this PR will require further and more thorough review by the maintainers team. The reason being this section of code you're working on is literally 15 to 20 years old, hasn't changed in a very long time, and used by hundreds of thousands of people every day for critical processes. Every tiny little change here could cause unforeseen problems in places we haven't thought of. |
|
Thank you for the review! Indeed, there is a significant risk of unforeseen problem. I found the blank node ID generation one but there might be others. |
|
@Tpt |
|
@ashleysommer Thank you so much! |
Fixes #1141
Proposed Changes