-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
gh-117056: fix discrepancy in _decimal/_pydecimal public signatures #137697
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
Conversation
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 is unusual to make only self
parameter positional-only (unless it conflicts with var-keyword parameter). This looks like code churn. In any case, pydoc
removes it, so it doesn't affect its output. I suggest to remove /
after self
, but leave it for other parameters. It is perhaps fine to keep it in dunder methods.
Lib/_pydecimal.py
Outdated
@@ -4134,10 +4134,10 @@ def add(self, a, b): | |||
else: | |||
return r | |||
|
|||
def _apply(self, a): | |||
def _apply(self, a, /): |
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.
This is not a public method.
Does it? >>> class A:
... def foo(self, /):
... pass
... def bar(self):
... pass
...
>>> help(A)
Help on class A in module __main__:
class A(builtins.object)
| Methods defined here:
|
| bar(self)
|
| foo(self, /)
|
| ----------------------------------------------------------------------
| Data descriptors defined here:
|
| __dict__
| dictionary for instance variables
|
| __weakref__
| list of weak references to the object
>>> help(A.foo)
Help on function foo in module __main__:
foo(self, /)
>>> help(A.bar)
Help on function bar in module __main__:
bar(self)
It's removed in the Python sphinx docs, yes, together with self parameter. (By hand. Or using autodoc, e.g. as this toy extension: https://python-gmp.readthedocs.io/en/latest/) |
You are right. But this does not affect common use. BTW, please update also the documentation if all these parameters are not already marked as positional-only. |
I agree with Serhiy, I think the |
Huh, distracting "/" will anyway appear in the help() output or in the inspect.signature()'s. Per default (C implementation). I think people who read sources will be less surprised by usual python syntax. And it's just a matter of time when they will come with new bug about incompatible API.
There is a ready for review pr: #131990. I hope it does all on sphinx side. |
|
Then lets close this pr (and, perhaps, issue as well). If API of _pydecimal and _decimal will be incompatible - it's not better than current state of art. Perhaps, issue will be solved later, when core devs will end war with Python syntax. |
_decimal
and_pydecimal
compatibility differences #117056