Skip to content

Conversation

@progval
Copy link
Contributor

@progval progval commented Jul 5, 2024

Summary of changes

Creating 'Collection(graph, RDF.nil)' used to add a 'rdf:nil rdf:rest rdf:nil.' triple to the graph, which turned the empty list into an infinite list.

This was a side-effect of unconditionally appending 'seq' to the collection, which is not possible when the collection is 'nil' without ill side-effects.

This commit makes the append conditional; and also checks the collection is non-empty in case the user explicitly tries to add item to the empty list

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Checked that all tests and type checking passes.
  • If the change has a potential impact on users of this project:
    • Added or updated tests that fail without the change.
    • Updated relevant documentation to avoid inaccuracies.
    • Considered adding additional documentation.
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.
Creating 'Collection(graph, RDF.nil)' used to add a 'rdf:nil rdf:rest rdf:nil.' triple to the graph, which turned the empty list into an infinite list. This was a side-effect of unconditionally appending 'seq' to the collection, which is not possible when the collection is 'nil' without ill side-effects. This commit makes the append conditional; and also checks the collection is non-empty in case the user explicitly tries to add item to the empty list.
@progval progval force-pushed the collection-empty-list branch from 3fa261d to fc02817 Compare July 5, 2024 11:37
@progval progval changed the title fix: Prevent Collection from add 'rdf:nil rdf:rest rdf:nil.' triples fix: Prevent Collection from adding 'rdf:nil rdf:rest rdf:nil.' triples Jul 15, 2024
This will allow using _end() in other methods that would not need to append
@nicholascar nicholascar added the fix Fixes an issue label Jul 24, 2024
@nicholascar nicholascar changed the title fix: Prevent Collection from adding 'rdf:nil rdf:rest rdf:nil.' triples Prevent Collection from adding 'rdf:nil rdf:rest rdf:nil.' triples Jul 24, 2024
@nicholascar nicholascar added the awaiting feedback More feedback is needed from the author of the PR or Issue. label Jul 24, 2024
@coveralls
Copy link

coveralls commented Jul 25, 2024

Coverage Status

coverage: 90.635%. first build
when pulling 6e65136 on progval:collection-empty-list
into 46695eb on RDFLib:main.

Co-authored-by: Ashley Sommer <ashleysommer@gmail.com>
@nicholascar nicholascar removed the awaiting feedback More feedback is needed from the author of the PR or Issue. label Jul 26, 2024
@ashleysommer ashleysommer mentioned this pull request Jul 27, 2024
3 tasks
@ashleysommer
Copy link
Contributor

There's an odd failing test now, weirdly its the same SPARQL test that is failing in #2796 that is completely unrelated to this PR.

@ashleysommer
Copy link
Contributor

That sporadic test is no longer failing, this can merge.

@ashleysommer ashleysommer merged commit cb2c8d1 into RDFLib:main Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Fixes an issue

4 participants