Skip to content
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

partial now behaves like a method descriptor #125983

Open
MegaIng opened this issue Oct 25, 2024 · 4 comments
Open

partial now behaves like a method descriptor #125983

MegaIng opened this issue Oct 25, 2024 · 4 comments
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir pending The issue will be closed if no feedback is provided stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@MegaIng
Copy link

MegaIng commented Oct 25, 2024

Bug report

Bug description:

This is an intentional change done in #121089, following #121027. That PR disregarded PEP-387 and breaks compatibility with less than a version warning (less than because the FutureWarning that was added was late in the python 3.13 release cycle)

I don't know if there is much more to add to this bug report - this change should not have been done without a proper deprecation period and IMO a proper discussion outside of two github issues. It should be reverted for 3.14 and a deprecation warning be added instead, with the final change, if being done at all, the earliest in 3.16.

It also already caused other issues, at least #125316. The reason I noticed this is because it broke actual tests on a project I am maintainer on: lark-parser/lark#1480

CPython versions tested on:

3.14

Operating systems tested on:

Windows

@MegaIng MegaIng added the type-bug An unexpected behavior, bug, or error label Oct 25, 2024
@Eclips4
Copy link
Member

Eclips4 commented Oct 25, 2024

cc @serhiy-storchaka @rhettinger

@serhiy-storchaka
Copy link
Member

The reason why this change does not follow the letter of PEP-387 is because this was impossible. The added warning itself causes a breaking change. #125316 was caused not by this change, but by the code necessary to produce a warning. This is a catch-22 situation -- to produce a warning about the future turn of partial() into descriptor, it should be turned into descriptor, and this breaks some code that does not use the descriptor protocol, but only checks if the object is a descriptor. Such code needs a special handling of the partial object. Several such places was fixed in the stdlib, but we do not want to keep such problematic code any longer. Alternatives to this are:

  • prolong the warning period, during which some code is broken, without bringing the benefits of the final change.
  • remove the warning, making the change more abrupt for most users.

@MegaIng
Copy link
Author

MegaIng commented Oct 25, 2024

While true that adding a warning itself is already a breaking change, actually implementing the behavior is an additional, different breaking change for people who relied on the behavior on partial - E.g. lark. IMO it would be better to just produce a warning for now and mirror the behavior of before.

Or even better, don't introduce a random, unnecessary breaking change whos only mentioned benefit is keeping an already fundamentally flawed comparison alive a bit more.

The fact that we have tests that are broken by this is not random - we actually find the behavior of partial as a non-method descriptor useful. And partialmethod exists - there is no functional need for partial to behave like a method, and in fact it would be more useful if it didn't.

@dg-pb
Copy link
Contributor

dg-pb commented Oct 25, 2024

And partialmethod exists - there is no functional need for partial to behave like a method, and in fact it would be more useful if it didn't.

Why is that? The former does not necessarily imply the latter.

It was decided for partial to behave as an ordinary function, i.e. as descriptor. This was done to bring lambda/def and partial mental models in line.

IMO it would be better to just produce a warning for now and mirror the behavior of before.

I appreciate the inconvenience. But given the situation at hand, which is for partial to behave as descriptor and the fact that it was already merged, there are 2 options:
a) Rewind and extend deprecation period
b) Adapt to it by using staticmethod

(a) would be a fair amount of inconvenience and disruption in development pipeline in this direction.

Does (b) have any complications beyond straight forward adaptation by wrapping in staticmethod?

@picnixz picnixz added stdlib Python modules in the Lib dir extension-modules C modules in the Modules dir pending The issue will be closed if no feedback is provided 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir pending The issue will be closed if no feedback is provided stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants