Conversation
src/spdx/parser/rdf/rdf_parser.py
Outdated
| for parent_node, _, element_node in graph.triples(triple): | ||
| try: | ||
| relationship = parse_implicit_relationship( | ||
| element_node, graph, parent_node, creation_info.document_namespace, relationship_type |
There was a problem hiding this comment.
In this line PyCharm complains about a wrong type: "Expected type 'URIRef', got 'Node' instead". I am not sure how to solve this, the return type of graph.triples(..) is Tuple[Node, Node, Node] which is pretty general. parse_implicit_relationship expects a URIRef which should be fulfilled if the rdf file is valid. The only solution I could think of is to accept a Node as input and check if element_node is a URIRef in parse_implicit_relationship and log an error if this is not the case. Any other suggestions on how to fix this are much appreciated.
There was a problem hiding this comment.
I think your proposal is the right way to go. We can't expect a "random" triple from the parsed RDF graph to be a URIRef, so we have to make sure that we get the correct type. I'm curious, though, why this doesn't pop up in other uses of graph.triples().
There was a problem hiding this comment.
I added a wrapper method for the graph.triples(..) and also for graph.value(..) to ensure types at some places. I decided to write a Warning in the logger if the type doesn't match as the code would probably run either way.
armintaenzertng
left a comment
armintaenzertng
left a comment
There was a problem hiding this comment.
Thanks for spotting and fixing this! :)
I have a few small remarks.
src/spdx/parser/rdf/rdf_parser.py
Outdated
| for parent_node, _, element_node in graph.triples(triple): | ||
| try: | ||
| relationship = parse_implicit_relationship( | ||
| element_node, graph, parent_node, creation_info.document_namespace, relationship_type |
There was a problem hiding this comment.
I think your proposal is the right way to go. We can't expect a "random" triple from the parsed RDF graph to be a URIRef, so we have to make sure that we get the correct type. I'm curious, though, why this doesn't pop up in other uses of graph.triples().
armintaenzertng
left a comment
armintaenzertng
left a comment
There was a problem hiding this comment.
Some comments on the new triples functions
armintaenzertng
left a comment
armintaenzertng
left a comment
There was a problem hiding this comment.
Looks good now :)
…d "describesPackage" relationships Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
meretp
force-pushed
the
issue_561_fix_rdf_parser
branch
from
f8115a6 to
af1dbfc
Compare
2 participants
This PR adds some logic to parse implicit "hasFiles" and "describesPackage" relationships.
fixes #561