Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
PEP 737: gh-111696: Add type.__fully_qualified_name__ attribute#112133
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
498a4d6
tof9a8c31
CompareExisting logic to format a type name as I found a lot of code like:
stype=self.exc_type.__qualname__smod=self.exc_type.__module__ifsmodnotin ("__main__","builtins"):ifnotisinstance(smod,str):smod="<unknown>"stype=smod+'.'+stype
def_type_repr(obj): ...ifisinstance(obj,type):ifobj.__module__=='builtins':returnobj.__qualname__returnf'{obj.__module__}.{obj.__qualname__}' ...
defstrclass(cls):return"%s.%s"% (cls.__module__,cls.__qualname__) |
I added a second commit using the new |
Doc/c-api/type.rst Outdated
@@ -185,6 +185,13 @@ Type Objects | |||
.. versionadded:: 3.11 | |||
.. c:function:: PyObject* PyType_GetFullyQualName(PyTypeObject *type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why can't it just be GetFullyQualifiedName?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We already have aPyType_GetQualName()
function which returnstype.__qualname__
:https://docs.python.org/dev/c-api/type.html#c.PyType_GetQualName
I followed thePyType_GetQualName()
name:PyType_GetFullyQualName()
returnstype.__fullyqualname__
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I know that, but it still tickles my grammar bone. :-(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok, I renamed the function toPyType_GetFullyQualifiedName()
.
I prefer to keep__fullyqualname__
for the attribute name. We already have__qualname__
. And since it's commonly used, I prefer to make it short.
An alternative is to use FQN acronym :-D
type.__fqn__
PyType_GetFQN()
But I prefer "longer" names, so I don't need to check the doc to know what they mean ;-)
On the Internet, Domain Name Service (DNS) useFQDN: Fully Qualified Domain Name :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I share Guido's unease with "fullyqualname": it sounds wrong to have the abbreviated "qual" between two unabbreviated words.
For comparison:
- Several dunders, including some of the most recently added ones, include an underscore in the word:
__class_getitem__
,__release_buffer__
,__type_params__
,__init_subclass__
,__text_signature__
- Other dunders include multiple words, often abbreviated, without underscores:
__getnewargs__
,__kwdefaults__
,__isabstractmethod__
I would prefer__fully_qualified_name__
. It's on the long side, but it matches the current name for the concept (mentioned inhttps://docs.python.org/3.12/glossary.html#term-qualified-name).__fqn__
seems too cryptic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I concur with Jelle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok, I renamed the attribute to__fully_qualified_name__
. Does it look better to you?
It's on the long side, but it matches the current name for the concept (mentioned inhttps://docs.python.org/3.12/glossary.html#term-qualified-name).fqn seems too cryptic.
I agree with you.
Add PyType_GetFullyQualifiedName() function with documentation andtests.
19c3bf6
to82dd956
CompareNaming is hard. Taking care of myself is harder. I have a lot going on at the moment and I really would prefer not to be responsible for this particular naming decision. Hopefully one or more of the other reviewers can help instead. |
IMO, this should use something more customizable than an attribute. A |
@@ -573,19 +573,19 @@ static PyObject * | |||
test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored)) | |||
{ | |||
PyObject *tp_name = PyType_GetName(&PyLong_Type); | |||
assert(strcmp(PyUnicode_AsUTF8(tp_name), "int") == 0); | |||
assert(PyUnicode_EqualToUTF8(tp_name, "int")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Even though it's still useful, this has nothing to do with the PR title and should go into a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's related, I changed the 3 tests which check the 3 C functions to get a "type name". I prefer to make the code consistent to make it easier to maintain.
This change also fix a bug: caller must check if PyUnicode_AsUTF8() is NULL. PyUnicode_EqualToUTF8() doesn't have this problem.
Lib/typing.py Outdated
if obj.__module__ == 'builtins': | ||
return obj.__qualname__ | ||
return f'{obj.__module__}.{obj.__qualname__}' | ||
return obj.__fullyqualname__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You are changing the contract for the method here. This may result in breaking code relying on previous behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think that you misunderstood the API of this attribute. Please check its documentation inDoc/library/stdtypes.rst
, or just try my PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Functional changes should not go into this PR.
rtn = PyUnicode_FromFormat("<class '%s'>", type->tp_name); | ||
Py_XDECREF(mod); | ||
PyObject *result = PyUnicode_FromFormat("<class '%U'>", name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same comment as above: you are readding "builtins." to builtin types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The fully qualified name omits the module if the module is "builtins". There is no behavior change. This code has multiple tests. I'm not sure that I get your comment correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As you can see from the code, the "builtins." part was deliberately omitted for builtin types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure of what you are talking about. You are commentingtype_repr()
function. I didn't change the function output, it's the same:
>>> import builtins>>> print(repr(builtins.str)) # <=== HERE "builtins." is omitted<class 'str'>>>> builtins.str.__module__'builtins'>>> builtins.str.__qualname__'str'
repr()
omits the module if it's "builtins". But it's added otherwise:
>>> import collections>>> print(repr(collections.OrderedDict))<class 'collections.OrderedDict'>>>> collections.OrderedDict.__module__'collections'>>> collections.OrderedDict.__qualname__'OrderedDict'
It's the same behavior than before.
He he, I totally get it and it's perfectly fine. Take care. |
This change is notexclusive with extending unittest example from this PR:
is replaced with:
I like the ability to get an attribute rather than having to build a f-string for that. Adding new formats to format a type name in Python has been discussed in length. Most recent to oldest discussions:
Problems:
So my plan is now to add |
Yes. Did any of those discussions reach consensus on adding |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Objects/typeobject.c Outdated
PyObject * | ||
PyType_GetFullyQualifiedName(PyTypeObject *type) | ||
{ | ||
return type_get_fullyqualname(type, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
returntype_get_fullyqualname(type,NULL); | |
returntype_fullyqualname(type,0); |
No need to call the intermediate function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I reorganized the calls. The special function withint is_repr
parameter now gets a_impl
suffix. The getter lostsget_
in its name to look alike type_name() and type_qualname() getters. It now calls PyType_GetFullyQualifiedName(). Does it look better to you?
@JelleZijlstra: Please review the updated PR. I tried to address all comments of your latest review. |
I wrotePEP 737 – Unify type name formatting for these changes: seethe PEP discussion. |
Thanks for taking the time to write a PEP. I think it's important that changes to builtin classes like this go through the PEP process :-) |
PEP 737 changed the API since this PR was created. I close this PR for now and will create a new one (or maybe reopen this PR) since PEP 737 will be approved. |
The Steering Council rejected the |
Uh oh!
There was an error while loading.Please reload this page.
Add PyType_GetFullyQualName() function with documentation and tests.
📚 Documentation preview 📚:https://cpython-previews--112133.org.readthedocs.build/