Skip to content

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

Closed
wants to merge 3 commits into from

Conversation

skirpichev
Copy link
Contributor

@skirpichev skirpichev commented Aug 13, 2025

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a 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.

@@ -4134,10 +4134,10 @@ def add(self, a, b):
else:
return r

def _apply(self, a):
def _apply(self, a, /):
Copy link
Member

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.

@skirpichev
Copy link
Contributor Author

In any case, pydoc removes it, so it doesn't affect its output.

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/)

@serhiy-storchaka
Copy link
Member

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.

@AA-Turner
Copy link
Member

I agree with Serhiy, I think the self, / is distracting.

@skirpichev
Copy link
Contributor Author

But this does not affect common use.

I agree with Serhiy, I think the self, / is distracting.

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.

BTW, please update also the documentation if all these parameters are not already marked as positional-only.

There is a ready for review pr: #131990. I hope it does all on sphinx side.

@AA-Turner
Copy link
Member

def f(self, /, ...) is generally only used when self might be a valid keyword argument. It isn't needed here, please revert the changes.

@skirpichev
Copy link
Contributor Author

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.

@skirpichev skirpichev closed this Aug 18, 2025
@skirpichev skirpichev deleted the fix-pydecimal-sigs/117056 branch August 18, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants