Skip to content

BuiltinFunctionType and BuiltinMethodType are different #3100

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.

Sign up for GitHub

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

Jump to bottom
Closed
Tetramad opened this issue Sep 21, 2021 · 8 comments
Closed

BuiltinFunctionType and BuiltinMethodType are different #3100

Tetramad opened this issue Sep 21, 2021 · 8 comments
Labels
C-compat A discrepancy between RustPython and CPython z-ca-2021 Tag to track contrubution-academy 2021

Comments

@Tetramad
Copy link
Contributor

Feature

types.BuiltinFunctionType must be equal to types.BuiltinMethodType.
In other word, types returned by builtin function and types returned by builtin method must be of the same type.

In CPython

>>> import types
>>> types.BuiltinFunctionType == types.BuiltinMethodType
True
>>> type(len) == type([].append)
True

In RustPython

>>>>> import types
>>>>> types.BuiltinFunctionType == types.BuiltinMethodType
False
>>>>> type(len) == type([].append)
False

Python Documentation

BuiltinFunctionType and BuiltinMethodType have same descriptions.
Also, commented at source code.
@DimitrisJim
Copy link
Member

Note: I think must be equal is a bit too strong in this case, I don't believe there's a requirement that these be equal. It would be nice if they would, though.

@DimitrisJim DimitrisJim added the C-compat A discrepancy between RustPython and CPython label Sep 21, 2021
@DimitrisJim DimitrisJim changed the title compatible issue: BuiltinFunctionType and BuiltinMethodType are different BuiltinFunctionType and BuiltinMethodType are different Sep 21, 2021
@Tetramad
Copy link
Contributor Author

Sorry for my bad english. I can't find words other than 'must be', 'should be'.
"It would be nice if they would", that's what I wanted to say. ;)

@youknowone
Copy link
Member

Roughly, adding zelf: Option<PyObjectRef> to PyBuiltinFunction and replacing PyBoundMethod to PyBuiltinFunction will solve it.

It looks like a good next-step issue for who understand how python binding works and finished a few good first issues.

@youknowone youknowone added the z-ca-2021 Tag to track contrubution-academy 2021 label Oct 16, 2021
@Snowapril
Copy link
Contributor

Snowapril commented Jul 6, 2022 •
edited
Loading

@youknowone This issue seems fixed.

>>>>> import types
>>>>> types.BuiltinFunctionType == types.BuiltinMeth
odType
False
>>>>> type(len) == type([].append)
False

Oh sorry, my mistake

@moreal
Copy link
Contributor

moreal commented Jul 25, 2022

I'm working on this issue. I left this comment to avoid task conflicts.

@seungha-kim
Copy link
Contributor

I'll take a look!

@seungha-kim seungha-kim mentioned this issue Mar 13, 2023
Closed
@youknowone
Copy link
Member

Maybe, we can adapt the optimization idea but not about the exact types.
CPython internally uses PyCMethod but expose it as PyCFunction. I think that will be very tricky in rust.

@youknowone
Copy link
Member

Partially resolved by #4873
I decide not to strictly follow CPython implementation for this behavior. We expose BuiltinMethodType to user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-compat A discrepancy between RustPython and CPython z-ca-2021 Tag to track contrubution-academy 2021
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Draft] Make builtin method type same as that of builtin function seungha-kim/RustPython
6 participants
@youknowone @seungha-kim @DimitrisJim @Snowapril @moreal @Tetramad