Skip to content
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

Changes to graph.serialize() #1183

Merged
merged 2 commits into from
Dec 27, 2020
Merged

Changes to graph.serialize() #1183

merged 2 commits into from
Dec 27, 2020

Conversation

ashleysommer
Copy link
Contributor

@ashleysommer ashleysommer commented Oct 8, 2020

This is part of our work to simplify and modernize the methods on rdflib.Graph to clean up the API surface of the Graph.

These are some long-overdue changes to serialize(). This is a slightly breaking change, so will be coming as part of RDFLib 6.0.0, but is done in such a way as to minimize breakage as much as possible.

These changes are:

  1. Change default format from 'xml' to 'turtle'
  2. If serialize() destination=None and encoding=None, output is a str, not bytes
  3. Add a new convenience method to print the Graph contents (turtle by default)
  • eg to print the Graph as trig format: g.print(format="trig")

Lots of fixes in tests to adapt to these changes (serialize is used a lot to check graph state in tests).
Blacked files which were involved in this change.

1) Change default format from 'xml' to 'turtle'
2) If destination is None and encoding is None, output a str, not bytes
3) Add a new convenience method to print the Graph contents (turtle by default)
Lots of fixes in tests to adapt to these changes (serialize is used a lot to check graph state in tests).
Blacked files which were involved in this change.
@ashleysommer
Copy link
Contributor Author

Note after these changes you can still get bytes output from serialize() if you pass it an encoding.

Eg:
g.serialize(format="xml") will give you str but
g.serialize(format="xml", encoding="utf-8") will give you bytes encoded as 'utf-8' and
g.serialize(format="xml", encoding="latin-1") will give you bytes encoded as 'latin-1' as expected

@nicholascar nicholascar self-requested a review December 17, 2020 19:11
@nicholascar
Copy link
Member

Just made a small edit to allow doctests to pass, awaiting Travis result

@coveralls
Copy link

coveralls commented Dec 17, 2020

Coverage Status

Coverage increased (+0.08%) to 75.505% when pulling dd1a7b1 on serialize1 into 7a53c61 on master.

@nicholascar nicholascar merged commit 3343a14 into master Dec 27, 2020
@nicholascar nicholascar deleted the serialize1 branch December 27, 2020 11:09
@white-gecko white-gecko added this to the rdflib 6.0.0 milestone Dec 28, 2020
@white-gecko
Copy link
Member

I understand the idea and I like that it is made easier now that a user does not need to ncessarily deal with the encoding, but having utf-8 per default. On the other hand I think it might confuse some users if adding the encoding parameter does not bring a string anymore but byte instead. This leads me to the more general question, why do we use ByteIO for the streams, why not StringIO instead?

@nicholascar
Copy link
Member

it might confuse some users if adding the encoding parameter does not bring a string anymore but byte instead

Yes it might the first time, but this will be one of the changelog notes for 6.0.0, so they will be notified and hopefully only see the issue the first time.

why do we use ByteIO for the streams, why not StringIO instead?

Perhaps just due to a lack of consistent development? Would you like to convert across to StringIO everywhere? @ashleysommer, is that sensible? Sounds fine to me to have string as default unless it's slow for large datasets. Can always be converted back

@white-gecko white-gecko removed their request for review January 2, 2021 13:12
drahnreb added a commit to drahnreb/pyLODE that referenced this pull request Aug 4, 2021
Fix AttributeError of serialize(). With backwards compatibility for
rdflib==5.0.0, as its the last python 2 release and as long as pyLODE
might still support rdflib 5.0.0
nicholascar added a commit to RDFLib/pyLODE that referenced this pull request Aug 11, 2021
mwatts15 added a commit to openworm/owmeta that referenced this pull request Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants