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

Add IdentifiedNode abstract intermediary class #1680

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

ajnelson-nist
Copy link
Contributor

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>
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 good to me, thanks for the change.

Copy link
Member

@nicholascar nicholascar left a comment

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.

@nicholascar nicholascar merged commit 3dbc848 into RDFLib:master Jan 20, 2022
@aucampia
Copy link
Member

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.

@ajnelson-nist ajnelson-nist deleted the add_identified_node branch January 21, 2022 15:33
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.
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.

Name for union of URIRef and BNode?
3 participants