Skip to content

Conversation

aucampia
Copy link
Member

@aucampia aucampia commented Aug 19, 2022

Summary of changes

The TriG serializer was only considering BNode references inside a single
graph and not counting the BNodes subjects as references when considering if a
BNode should be serialized as unlabeled blank nodes (i.e. [ ]), and as a
result it was serializing BNodes as unlabeled if they were in fact referencing
BNodes in other graphs.

One caveat of this change is that some RDF Datasets may be serialized
less sussinctly in that it would not use unlabeled blank nodes where it is
technically possible. This can be trivially fixed, but a trivial fix
increases the compuational complexity of serialization significantly.

Other changes:

  • Removed the roundtrip xfail that this change fixed.
  • Added another roundtrip test which has various combinations of BNode
    references across graphs in a dataset, this test fails for JSON-LD
    however, so while this change removes one xfail it also now adds
    another.
  • Set the default indent_size and style in .editorconfig as to avoid
    relying on undefined system defaults.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Added tests for any changes that have a runtime impact.
  • Checked that all tests and type checking passes.
  • For changes that have a potential impact on users of this project:
    • Considered updating our changelog (CHANGELOG.md).
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

@aucampia aucampia force-pushed the iwana-20220818T2117-fix_trig_bname branch 3 times, most recently from 0654704 to 8e1cb59 Compare August 19, 2022 20:46
@coveralls
Copy link

coveralls commented Aug 19, 2022

Coverage Status

Coverage decreased (-0.005%) to 90.627% when pulling a72eeaf on aucampia:iwana-20220818T2117-fix_trig_bname into 04bf774 on RDFLib:master.

@aucampia aucampia force-pushed the iwana-20220818T2117-fix_trig_bname branch 2 times, most recently from eaae188 to 44bc096 Compare August 19, 2022 21:05
@aucampia aucampia marked this pull request as ready for review August 19, 2022 21:29
@aucampia aucampia requested a review from a team August 19, 2022 21:29
@aucampia aucampia added the review wanted This indicates that the PR is ready for review label Aug 19, 2022
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

a-ha, well spotted

@aucampia aucampia force-pushed the iwana-20220818T2117-fix_trig_bname branch 2 times, most recently from 34d5ac1 to d2eb9e4 Compare August 23, 2022 21:42
The TriG serializer was only considering BNode references inside a single
graph and not counting the BNodes subjects as references when considering if a
BNode should be serialized as unlabeled blank nodes (i.e. `[ ]`), and as a
result it was serializing BNodes as unlabeled if they were in fact referencing
BNodes in other graphs.

One caveat of this change is that some RDF Datasets may be serialized
less sussinctly in that it would not use blank nodes where it is
technically possible. This can be trivially fixed, but a trivial fix
increases the compuational complexity of serialization significantly.

Other changes:
- Removed the roundtrip xfail that this change fixed.
- Added another roundtrip test which has various combinations of BNode
  references across graphs in a dataset, this test fails for JSON-LD
  however, so while this change removes one xfail it also now adds
  another.
- Set the default indent_size and style in .editorconfig as to avoid
  relying on undefined system defaults.
@aucampia aucampia force-pushed the iwana-20220818T2117-fix_trig_bname branch from d2eb9e4 to a72eeaf Compare August 24, 2022 18:09
@aucampia aucampia merged commit 8a58ae8 into RDFLib:master Aug 24, 2022
@aucampia aucampia deleted the iwana-20220818T2117-fix_trig_bname branch April 9, 2023 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review wanted This indicates that the PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants