-
Notifications
You must be signed in to change notification settings - Fork 555
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
Name for union of URIRef and BNode? #1526
Comments
SHACL's Unless we have alternative logic - I don't think we do - we should adopt parallel classes/categories |
Ah ha, I see you already referenced these SHACL classes in PR #1515! So to answer more directly:
RDF calls non-literal, local IDd, nodes blank nodes and thinsk with URIs named nodes of course. Extending on this, HexTuples calls BN nodes localIds and URI nodes globalId nodes. So I think we can go with |
Once the name is settled, what is the maintainer preference for how to implement this - an abstract class, or a |
Yes, the URI/IRI etc. naming issues motivated I prefer I don't know all the pros and cons of |
However, I think we should be careful in naming the Unions so that we don't consume names with Unions that we may later want to use for runtime types or normal classes, if we make So in summary, my preference:
|
Let's go with @aucampia's suggestion above. I think that all of the type checking that @ajnelson-nist wants can be done with
But just a check: we already have a class My initial test with |
One other point in favor of a subclass, instead of a I'd also wondered why |
It’s not a bug but it is a strange - in my opinion - design choice! I suspect it was just easier to implement than what we are talking about now. There are many cases too in RDFlib where a patter was implemented and then broken by others later, so we may be seeing that here. We can definitely improve this! |
I think |
Thanks @niklasl. I think we should probably implement the change suggested here and definitely draw up the Python class model here in UML for RDFLib documentation. Better to have these structures more obvious to people |
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. This patch addresses at least some of Issue 1526. References: * RDFLib#1526 Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
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>
PR 1680 filed. I don't quite understand how Also, if you want a UML diagram to be included in the PR, I'm not sure where you'd like that. Anyone, please feel free to push on top of the PR's branch. I have the "Allow edits and access to secrets by maintainers" box checked, so please let me know if that doesn't quite work out. |
I think we can do the diagram separately, not sure variable should inherit from the identified node, but I may be wrong about it. |
Yes, the point here is to single out the "real" Some notes:
I personally did find the naming of "nodes" ( (I may prefer the names Possibly though, from this, I retain a slight preference on my part for Unfortunately though, nor does Perhaps |
Regarding the diagram, I think the best option is to add https://github.com/sphinx-contrib/kroki so that we can put this in rst file: .. kroki::
@startuml
skinparam shadowing false
skinparam packageStyle rectangle
class Node
class Identifier {
eq(other)
neq(other)
startswith(prefix, start, end)
}
Identifier -up-|> Node
class BNode {
(...)
}
BNode -up-|> Identifier
class Literal {
(...)
}
Literal -up-|> Identifier
class URIRef {
(...)
}
URIRef -up-|> Identifier
class Variable {
(...)
}
Variable -up-|> Identifier
class Genid
Genid -up-|> URIRef
class RDFLibGenid
RDFLibGenid -up-|> Genid
@enduml And then get output like this in generated HTML: |
I will add this in a separate PR |
This looks like a nice enhancement @aucampia! I really do think a class diagram will make a big difference for understanding the toolkit and I'm surprise we didn't think of it before... |
The diagram is based on the diagram created by Graham Higgins (@gjhiggins) in RDFLib#1526 This shows the class heirarchy of various terms such as Identifier, IdentifiedNode, URIRef, Literal, etc. The diagram is in [plantuml](https://plantuml.com/class-diagram) and compiled to svg by the [kroki extension for sphinx](https://github.com/sphinx-contrib/kroki). Diagrams can be rendered from the plantuml at https://kroki.io/. Other changes: - Some updates to the "Writing RDFLib Documentation" page.
A follow-on patch will regenerate the Make-managed file. References: * RDFLib/rdflib#1526 Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
I frequently need to do type checks for
Node
s that are notLiteral
s, mainly to catch programming errors where the "Thing vs String" logic may have been lax. I'm casting around for a name that either could be defined in theNode
subclass hierarchy as an abstract class, or instead could be defined as aUnion
. My recollection is there isn't an obvious name from RDFS for this.@aucampia suggested
Subject
in this comment:I have a mental TODO item to review the definition of RDF Resource to see if that meets the need, but I'd like to see if someone has already found an appropriate term for "not-
Literal
".The text was updated successfully, but these errors were encountered: