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

Jump to bottom

Argument Clinic: inline parsing code for 1-argument functions #68055

Closed
serhiy-storchaka opened this issue Apr 4, 2015 · 10 comments
Closed

Argument Clinic: inline parsing code for 1-argument functions #68055

serhiy-storchaka opened this issue Apr 4, 2015 · 10 comments
Assignees
@serhiy-storchaka
Labels
3.8 only security fixes topic-argument-clinic type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 23867
Nosy @vstinner, @larryhastings, @serhiy-storchaka, @ammaraskar, @tirkarthi
PRs
  • bpo-23867: Argument Clinic: inline parsing code for a single positional parameter. #9689
    		•
    Dependencies
  • bpo-26305: Make Argument Clinic to generate PEP 7 conforming code
  • bpo-26902: Argument Clinic incorrectly works with custom converter and renamed parameter
    		•
    Files
  • clinic_meth_o_inline.patch
  • clinic_meth_o_inline.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2018-12-25.14:57:41.640>
created_at = <Date 2015-04-04.14:24:36.243>
labels = ['type-feature', '3.8', 'expert-argument-clinic']
title = 'Argument Clinic: inline parsing code for 1-argument functions'
updated_at = <Date 2019-01-04.13:52:43.874>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2019-01-04.13:52:43.874>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2018-12-25.14:57:41.640>
closer = 'serhiy.storchaka'
components = ['Argument Clinic']
creation = <Date 2015-04-04.14:24:36.243>
creator = 'serhiy.storchaka'
dependencies = ['26305', '26902']
files = ['38830', '42672']
hgrepos = []
issue_num = 23867
keywords = ['patch']
message_count = 10.0
messages = ['240074', '264596', '264629', '264631', '286788', '326979', '327493', '332507', '332511', '332978']
nosy_count = 5.0
nosy_names = ['vstinner', 'larry', 'serhiy.storchaka', 'ammar2', 'xtreak']
pr_nums = ['9689']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue23867'
versions = ['Python 3.8']
    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch makes Argument Clinic to inline parsing code for most popular formats in functions with single positional argument. This makes parsing faster.

    @serhiy-storchaka serhiy-storchaka added topic-argument-clinic type-feature A feature request or enhancement labels Apr 4, 2015
    @serhiy-storchaka
    Copy link
    Member Author

    Synchronized with tip.

    @larryhastings
    Copy link
    Contributor

    Why is this dependent on bpo-26305?

    @serhiy-storchaka
    Copy link
    Member Author

    Because new generated code contains "if" statements and braces (and PEP-7 requires even more braces than current patch adds). The patch for bpo-26305 allows to repeat braces only twice instead of 4 times.

    @vstinner
    Copy link
    Member

    vstinner commented Feb 2, 2017

    I like the idea since I just proposed something similar in the issue bpo-29419, but it seems like your change removes the function name from error messages which can become much more obscure.

    Maybe we should wrap all exceptions into a new TypeError which contains at least the function name, or even the parameter name/position. I mean chained exception to keep the original exception which contains more information.

    The best would be to have all information in a single error message, but it is likely to be much more complex to implement, especially if you want to support arbitrary converter function, not only simple formats like i". So I think that two chained exceptions is a reasonable compromise. What do you think?

    If we succeed to get the function name and the parameter name or position, the error messages will be MUCH MORE better than currently! And Argument Clinic allows us to implement this feature.

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you all for your comments. Addressed all comments, fixed few other bugs, added support for more convertors. I think this patch is completed.

    This is just a first step. Next steps are adding support of multiple positional-only parameters, optional groups, and finally keyword parameters.

    @serhiy-storchaka serhiy-storchaka added the 3.8 only security fixes label Oct 3, 2018
    @serhiy-storchaka serhiy-storchaka self-assigned this Oct 9, 2018
    @vstinner
    Copy link
    Member

    I'm a little bit sad that the PR doesn't add new tests :-(

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 32d96a2 by Serhiy Storchaka in branch 'master':
    bpo-23867: Argument Clinic: inline parsing code for a single positional parameter. (GH-9689)
    32d96a2

    @serhiy-storchaka
    Copy link
    Member Author

    See bpo-35582 for next step.

    @vstinner
    Copy link
    Member

    vstinner commented Jan 4, 2019

    Nice optimization! I wanted to implement it, but then I forgot.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.8 only security fixes topic-argument-clinic type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @serhiy-storchaka @larryhastings