FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-117056: fix discrepancy in _decimal/_pydecimal public signatures by skirpichev · Pull Request #137697 · python/cpython · GitHub

/ cpython Public

gh-117056: fix discrepancy in _decimal/_pydecimal public signatures - #137697

Closed
skirpichev wants to merge 3 commits into
python:mainfrom
skirpichev:fix-pydecimal-sigs/117056
Closed

gh-117056: fix discrepancy in _decimal/_pydecimal public signatures#137697
skirpichev wants to merge 3 commits into
python:mainfrom
skirpichev:fix-pydecimal-sigs/117056

Conversation

skirpichev commented Aug 13, 2025
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread Lib/_pydecimal.py Outdated
return r

def _apply(self, a):
def _apply(self, a, /):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is not a public method.

Copy link
Copy Markdown
Member 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/)

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member 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.

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Member 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 closed this Aug 18, 2025
skirpichev deleted the fix-pydecimal-sigs/117056 branch August 18, 2025 12:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL