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

[BUG] __get__ does not allow optional last argument breaks Python compatibility #6527

Open
user202729 opened this issue Nov 30, 2024 · 3 comments

Comments

@user202729
Copy link

user202729 commented Nov 30, 2024

Describe the bug

As in the title.

Similar: #5160 (has been fixed now)

Code to reproduce the behaviour:

cdef class F:
	def __get__(self, a, b=None):
		print(b)

F().__get__(1)

Expected behaviour

Works well.

OS

Linux

Python version

3.11

Cython version

3.0.11 I believe?

Additional context

Side note, currently

cdef class F:
	def __get__(self, a, b):
		print(b)

is accepted, but in Cython you can call it omitting the last argument. This is currently used in SageMath code base (see sage/src/sage/misc/cachefunc.pyx), so making it raise error (for Python compatibility) would break SageMath.

I think it's still better to raise error when the code does not specify default argument though (to avoid confusing behavior), but a deprecation period is needed.

@scoder
Copy link
Contributor

scoder commented Nov 30, 2024

The second argument in the Sage code is unused, so Sage could easily be fixed by removing it.

Is there a practical reason to implement __get__ with two arguments?

@user202729
Copy link
Author

user202729 commented Nov 30, 2024

If I understood https://docs.python.org/3/reference/datamodel.html#object.__get__ correctly, it is recommended to implement __get__ in a way that it accepts either one or two arguments.

The second argument is (occasionally) useful when the instance is None --- namely if you do Foo.c, then the descriptor object is called __get__(descriptor object, None, Foo).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants