Skip to content

derive __text_signature__ from method definition - #2923

Merged
DimitrisJim merged 2 commits into
RustPython:masterfrom
youknowone:derive-text-signature
Aug 22, 2021
Merged

derive __text_signature__ from method definition#2923
DimitrisJim merged 2 commits into
RustPython:masterfrom
youknowone:derive-text-signature

Conversation

@youknowone

@youknowone youknowone commented Aug 21, 2021

Copy link
Copy Markdown
Member

This PR is not perfectly solving the issue but showing the first step to go. I think we can keep the signature and actual function better from this way.

improving #2904 in automated way

cc @moreal

@youknowone youknowone changed the title derive __text_signature__ from definition derive __text_signature__ from method definition Aug 21, 2021
@youknowone
youknowone force-pushed the derive-text-signature branch from 7d9bb27 to 374f263 Compare August 21, 2021 16:14
@youknowone
youknowone requested a review from DimitrisJim August 22, 2021 03:45
Comment thread derive/src/pyclass.rs Outdated
@DimitrisJim

DimitrisJim commented Aug 22, 2021

Copy link
Copy Markdown
Member

In general I think this is a step in the right direction. Many builtins will be covered and those that aren't covered can just have the signature placed there manually (i.e this step would be nice if it was made only if a signature hasnt been specified already).

Another small detail on this: it now requires us to try and follow the names used in CPython more closely (though I'm sure most functions/methods already do that).

@youknowone
youknowone force-pushed the derive-text-signature branch 2 times, most recently from be8790a to 65dd911 Compare August 22, 2021 04:38
@youknowone

Copy link
Copy Markdown
Member Author

Yes, that will be a new concern for argument names. Already generated object.__eq__ uses other as the argument name but doc refers value.

@youknowone
youknowone force-pushed the derive-text-signature branch from 65dd911 to 5ee9719 Compare August 22, 2021 08:11
Comment thread vm/src/builtins/object.rs
Comment on lines -17 to -19
/// object()
/// --
///

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understood, ImplItem<Item> for MethodItem will not affect at class documentation ClassItem. When I executed object.__text_signature__, it returns None. I guess these changes will revert some resolved problems again (#2732 (comment), #2623 (comment)).

@youknowone
youknowone force-pushed the derive-text-signature branch from 5ee9719 to da16618 Compare August 22, 2021 08:37
@DimitrisJim
DimitrisJim merged commit ac98e08 into RustPython:master Aug 22, 2021
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.

3 participants