-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added a fix for parsing rdflib literals. #229
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #229 +/- ##
==========================================
+ Coverage 78.41% 78.44% +0.02%
==========================================
Files 18 18
Lines 1696 1698 +2
==========================================
+ Hits 1330 1332 +2
Misses 366 366 ☔ View full report in Codecov by Sentry. |
# This will handle rdflib literals correctly and probably most other | ||
# literal representations as well. | ||
# This should handle rdflib literals correctly (and probably most other | ||
# literal representations as well) | ||
if hasattr(literal, "value"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But can a literal not have the attribute? Isn't it always there, but sometimes with value None? (I thought the reason for this fix is exactly that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, the argument name literal
is not very well chosen. It may be any Python object that can be converted to a literal (most of these do not have a value
attribute). It was changed by Casper from obj
to literal
in one of his early reviews. Should we revert?
Co-authored-by: Francesca L. Bleken <48128015+francescalb@users.noreply.github.com>
Description
We cannot use the
value
attribute of anrdflib.Literal
to access its value, since it isNone
for some datatypes (at least for rdf:JSON).Seems more robust to use the existence of the
n3()
method to test for a literal and the string representation to access its value.Type of change
Checklist for the reviewer
This checklist should be used as a help for the reviewer.