GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-117056: fix discrepancy in _decimal/_pydecimal public signatures by skirpichev · Pull Request #137697 · python/cpython

Conversation

@skirpichev

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: _decimal and _pydecimal compatibility differences #117056

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev
pythongh-117056: fix discrepancy in _decimal/_pydecimal public signat…
dac8e43
…ures
@skirpichev
skirpichev requested a review from AA-Turner August 13, 2025 04:29
@bedevere-app bedevere-app Bot mentioned this pull request Aug 13, 2025
_decimal and _pydecimal compatibility differences #117056
Closed
@skirpichev
skirpichev marked this pull request as ready for review August 13, 2025 05:00
@bedevere-app bedevere-app Bot added the awaiting review label Aug 13, 2025
This was referenced Aug 14, 2025
Sync-up docstrings in C version of the the decimal module #71966
Open
gh-73487: Convert _decimal to use Argument Clinic (part 2) #137637
Merged
serhiy-storchaka
serhiy-storchaka reviewed Aug 17, 2025
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Lib/_pydecimal.py Outdated
return r

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

@serhiy-storchaka serhiy-storchaka Aug 17, 2025

Copy link
Copy Markdown
Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@skirpichev

skirpichev commented Aug 17, 2025

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

skirpichev added 2 commits August 17, 2025 18:35
@skirpichev
Merge branch 'master' into fix-pydecimal-sigs/117056
a7aa921
@skirpichev
address review: revert private method
ec08dc1
@serhiy-storchaka

serhiy-storchaka commented Aug 17, 2025

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AA-Turner

AA-Turner commented Aug 17, 2025

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev

skirpichev commented Aug 17, 2025

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@AA-Turner

AA-Turner commented Aug 18, 2025

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@skirpichev

skirpichev commented Aug 18, 2025

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@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

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@AA-Turner AA-Turner Awaiting requested review from AA-Turner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@skirpichev @serhiy-storchaka @AA-Turner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.