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.

Sign up for GitHub

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

Jump to bottom

Add __dir__ to DefinedNamespaceMeta. #1626

Merged
merged 2 commits into from
Dec 28, 2021
Merged

Add __dir__ to DefinedNamespaceMeta. #1626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52dacaf
Add __dir__ to DefinedNamespaceMeta.
edmondchuc Dec 28, 2021
4dce6ff
Import Iterable from typing instead of collections.
edmondchuc Dec 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion rdflib/namespace/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import logging
import warnings
from typing import TYPE_CHECKING, List, Union
from typing import TYPE_CHECKING, List, Union, Iterable
from unicodedata import category

from pathlib import Path
Expand Down Expand Up @@ -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 •
edited
Loading

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

values = {cls[str(x)] for x in cls.__annotations__}
return values


class DefinedNamespace(metaclass=DefinedNamespaceMeta):
"""
Expand Down
35 changes: 35 additions & 0 deletions test/test_definednamespace_dir.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
from rdflib import RDF


def test_definednamespace_dir():
x = dir(RDF)

values = [
RDF.nil,
RDF.direction,
RDF.first,
RDF.language,
RDF.object,
RDF.predicate,
RDF.rest,
RDF.subject,
RDF.type,
RDF.value,
RDF.Alt,
RDF.Bag,
RDF.CompoundLiteral,
RDF.List,
RDF.Property,
RDF.Seq,
RDF.Statement,
RDF.HTML,
RDF.JSON,
RDF.PlainLiteral,
RDF.XMLLiteral,
RDF.langString,
]

assert len(values) == len(x)

for value in values:
assert value in x