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

Allow hext to participate in RDF format roundtripping #1656

Merged
merged 11 commits into from
Jan 15, 2022

Conversation

nicholascar
Copy link
Member

Until now, HexTuples parsing & serializing could not participate in RDF format roundtripping due to numerous failures. well, I've solved them by:

  • doing better JSON serializing
  • allowing the test_roundtrip.py functions to conflate "" with ""^^xsd:string, for hext only
  • skipping only 2 tests
  • allowing HexT parsing to treat a value "" as value, not None

Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, some suggestions but the only critical one is to manually specify the type of HextuplesSerializer.graph_type in rdflib/plugins/serializers/hext.py

@aucampia
Copy link
Member

aucampia commented Jan 9, 2022

Making a PR with all changes included

@aucampia
Copy link
Member

aucampia commented Jan 9, 2022

nicholascar and others added 7 commits January 10, 2022 00:55
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
@nicholascar
Copy link
Member Author

Thanks @aucampia. I added those casts to try and avoid some warnings shown in PyCharm but yeah, the code passed all MyPy tests without the casts. But then I did miss that type setting on graph_type as you saw! I used to be a C# developer once and all this Python typing takes me back to 2005...

…hext_fixes

One typing fix and removal of redundant casts
@nicholascar
Copy link
Member Author

I'll give this a few days but if there's no further review, I'll merge: it's a pretty small PR and has had one review!

@aucampia
Copy link
Member

Think you should go ahead with the merge as this fixes some important things in serialization.

Co-authored-by: Iwan Aucamp <aucampia@gmail.com>
@nicholascar nicholascar merged commit e8db150 into master Jan 15, 2022
@nicholascar nicholascar deleted the roundtrip_hext branch January 15, 2022 02:17
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.

2 participants