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 __dir__ to DefinedNamespaceMeta. #1626

Merged
merged 2 commits into from
Dec 28, 2021

Conversation

edmondchuc
Copy link
Contributor

Fixes #1593

Proposed Changes

  • Add __dir__ to rdflib.namespace.DefinedNamespaceMeta.

@@ -228,6 +228,10 @@ def __contains__(cls, item):
if issubclass(c, DefinedNamespace)
)

def __dir__(cls) -> Iterable[str]:
Copy link
Member

@aucampia aucampia Dec 28, 2021

Choose a reason for hiding this comment

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

Technically this breaks the interface, as anything that depended on the default dir implementation would break. Not sure if this is a serious concern, but something we should think about.

$ pipx run --spec rdflib==6.1.1 python3 -c 'from rdflib import RDF; print(dir(RDF))'
['_NS', '__annotations__', '__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_fail', '_underscore_num']

I highly doubt that this would be a problem for anyone though.

I'm fine with this approach and think it is likely best, but as a note to other reviewers; it would be good to think about other options, like providing an explicit method for it, thought that raises same concerns as before with name conflicts so that is likely worse.

Copy link
Member

Choose a reason for hiding this comment

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

I highly doubt that this would be a problem for anyone though.

Yes, I highly doubt this will be a problem too and I'm in favour of reporposing the well-known __dict__ for this, rather than adding a new method that people have to learn

@nicholascar nicholascar merged commit 8aa0aa0 into RDFLib:master Dec 28, 2021
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.

questions about DefinedNamespaces (OWL, RDFS, etc etc)
3 participants