-
Notifications
You must be signed in to change notification settings - Fork 558
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
Literal.toPython() support for xsd:hexBinary #388
Conversation
Now passing all the tests except one due to the nose last release nose-devs/nose#780 . This regression seems to be independent of this pull request and should affect the master branch as well. I was able to reproduce it and to prevent it by downgrading nose to 1.3.0. |
could you rebase this on top of master, should've fixed the problem with nose... |
(time, (lambda i:i.isoformat(), _XSD_TIME)), | ||
(xml.dom.minidom.Document, (_writeXML, _RDF_XMLLITERAL)), | ||
# Specific first | ||
((basestring, _XSD_HEXBINARY), (hexlify, _XSD_HEXBINARY)), |
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.
why can't this just be (basestring, (hexlify, _XSD_HEXBINARY)),
?
i guess i'll have to think about this a bit more...
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.
My point was to distinguish a regular string from a hexstring with the xsd:hexBinary datatype.
In this implementation I put specific rules first, before generic rules. The distinction between specific and generic rules is based on the datatype. Please note that the ordering of these rules is important.
We could use the more compressed structure you propose (and change _castPythonToLiteral()
accordingly) if we consider that no the function _castPythonToLiteral()
should never return a default datatype for a regular string.
Indeed, the rule (basestring, (None, _XSD_STRING))
would not be generic anymore while in with my approach ((basestring, None), (None, _XSD_STRING))
is still generic.
I think it is better to keep the distinction between the incoming and outcoming datatypes.
I rebased it and now all tests passed. Thanks |
This looks more or less ok to me - probably there will be other cases where it may be useful to have a special lexical form constructed based on a datatype. For example, you could write a rule for casting a datetime to just a date - slightly contrived example: from datetime import datetime
now = datetime.now()
l = Literal(now, datatype=XSD.date) Anyone else? Any reason not to make the literal-mapping rules more powerful? You'll have to update the bind method as well though: https://github.com/RDFLib/rdflib/blob/master/rdflib/term.py#L1493 and this: https://github.com/RDFLib/rdflib/blob/master/examples/custom_datatype.py |
Because of the For backward compatibility, I propose to change the def bind(datatype, pythontype, constructor=None, lexicalizer=None, datatype_specific=False) |
What should I change in the custom datatype example? To me, it is fine, it just shows a simple usage. |
This PR has been re-issued in PR #979 to cater for Python 2/3 differences and since the original repo has gone away. |
Following the discussion #386, here is an implementation of the bytes option, where
toPython()
returns a bytes value.