Add IdentifiedNode abstract intermediary class by ajnelson-nist · Pull Request #1680 · RDFLib/rdflib (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation3 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Fixes #1526

Proposed Changes

• Add IdentifiedNode to class hierarchy in term.py.
• Cite RDF section for class name, found from following up on @nicholascar 's name sleuthing.
• Expose IdentifiedNode symbol from /rdflib/__init__.py.
• Update unit test to use new symbol.

This patch adds and exposes an intermediary class IdentifiedNode as a superclass of URIRef and BNode. From review of the subclass methods for identical implementations, two appeared to be able to move into this superclass.

Thanks to Nicholas Car for finding this pragmatic name.

This patch addresses at least some of Issue 1526.

References:

• RDFLib#1526

Cc: Nicholas Car nicholas.car@surroundaustralia.com Signed-off-by: Alex Nelson alexander.nelson@nist.gov

Member

aucampia left a comment • Loading

Choose a reason for hiding this comment

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

Looks good to me, thanks for the change.

Choose a reason for hiding this comment

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

Looks good, and I think that the only tests failing are ok as these are the not finalised pre-commit hooks.

Looks good, and I think that the only tests failing are ok as these are the not finalised pre-commit hooks.

yes, I disabled them now and will re-enable once that PR is merged to avoid confusion from others, the PR for pre-commits has links to my fork which has it enabled for illustrative purposes.

ashleysommer added a commit that referenced this pull request

Aug 1, 2024

When PR #1680 was merged to add the IdentifiedNode intermediate class, it was missing the required n3() fn, and missed the __slots__ directive.

nicholascar pushed a commit that referenced this pull request

Aug 1, 2024

• Fix missing features of IdentifiedNode When PR #1680 was merged to add the IdentifiedNode intermediate class, it was missing the required n3() fn, and missed the __slots__ directive.

• Remove type-abstract suppressions for IdentifiedNode usages because its no longer considered abstract by the type checker.