Skip to content

[Draft] Make builtin method type same as that of builtin function - #4686

Closed
seungha-kim wants to merge 2 commits into
RustPython:mainfrom
seungha-kim:builtin-method-type
Closed

seungha-kim wants to merge 2 commits into
RustPython:mainfrom
seungha-kim:builtin-method-type

Conversation

@seungha-kim

Copy link
Copy Markdown
Contributor

Fixes #3100

Comment thread vm/src/function/builtin.rs Outdated
@seungha-kim

seungha-kim commented Mar 13, 2023 •

Copy link
Copy Markdown
Contributor Author

I missed that qualname of PyClassMethod must be considered too. (it is the cause of failing tests) Turning this PR to draft now and will look into PyClassMethod.

@seungha-kim
seungha-kim marked this pull request as draft March 13, 2023 13:53
@seungha-kim seungha-kim changed the title Make builtin method type same as that of builtin function [Draft] Make builtin method type same as that of builtin function Mar 14, 2023
@seungha-kim
seungha-kim force-pushed the builtin-method-type branch from 3a98980 to 6c3976f Compare March 14, 2023 10:14
@seungha-kim
seungha-kim force-pushed the builtin-method-type branch from 16a0863 to ffece09 Compare March 23, 2023 15:03
@youknowone
youknowone force-pushed the builtin-method-type branch from ffece09 to ea5e5ee Compare April 5, 2023 16:07
zelf: None,
value: self,
module: None,
is_classmethod: false,

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.

I am not sure about this field

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.

ah, i got it. we don't have PyMethodDef. So we cannot avoid it now.

@youknowone
youknowone force-pushed the builtin-method-type branch 13 times, most recently from ca23718 to b76eb6d Compare April 9, 2023 13:56
@youknowone
youknowone force-pushed the builtin-method-type branch from b76eb6d to 2e06739 Compare April 24, 2023 19:42
@youknowone

Copy link
Copy Markdown
Member

I made #4873 including this patch.
It took almost a month 😵

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BuiltinFunctionType and BuiltinMethodType are different

3 participants