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.

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

Simplify urllib.parse_qsl #88145

Closed
Cito mannequin opened this issue Apr 29, 2021 · 8 comments
Closed

Simplify urllib.parse_qsl #88145

Cito mannequin opened this issue Apr 29, 2021 · 8 comments
Labels
3.10 only security fixes 3.11 only security fixes performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@Cito
Copy link
Mannequin

Cito mannequin commented Apr 29, 2021

BPO 43979
Nosy @Cito, @corona10, @miss-islington, @robertohueso
PRs
  • bpo-43979: Remove unnecessary operation from urllib.parse.parse_qsl #25756
    		•

    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 = None
closed_at = <Date 2021-05-01.00:06:33.440>
created_at = <Date 2021-04-29.19:29:25.878>
labels = ['3.11', 'library', '3.10', 'performance']
title = 'Simplify urllib.parse_qsl'
updated_at = <Date 2021-05-01.00:06:39.801>
user = 'https://github.com/Cito'

    bugs.python.org fields:

    activity = <Date 2021-05-01.00:06:39.801>
actor = 'corona10'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-01.00:06:33.440>
closer = 'corona10'
components = ['Library (Lib)']
creation = <Date 2021-04-29.19:29:25.878>
creator = 'cito'
dependencies = []
files = []
hgrepos = []
issue_num = 43979
keywords = ['patch']
message_count = 8.0
messages = ['392345', '392369', '392486', '392489', '392490', '392491', '392492', '392504']
nosy_count = 4.0
nosy_names = ['cito', 'corona10', 'miss-islington', 'robertohueso']
pr_nums = ['25756']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue43979'
versions = ['Python 3.10', 'Python 3.11']
    @Cito
    Copy link
    Mannequin Author

    Cito mannequin commented Apr 29, 2021

    Just noticed the following code in urrlib.parse_qsl:

        pairs = [s1 for s1 in qs.split(separator)]
    for name_value in pairs:
        ...

    see

    cpython/Lib/urllib/parse.py

    Line 755 in 088a15c

    pairs = [s1 for s1 in qs.split(separator)]

    This looks like an unnecessary list comprehension to me, probably a relic of earlier code that used a nested list comprehension for splitting with two different separators.

    Can't we just do this instead now, which is faster and shorter?

       for name_value qs.split(separator):

    I can provide a PR if wanted.

    @Cito Cito mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir performance Performance or resource usage labels Apr 29, 2021
    @robertohueso
    Copy link
    Mannequin

    robertohueso mannequin commented Apr 30, 2021

    Seems like a nice simplification to me. Can I open PR? :)

    @corona10
    Copy link
    Member

    I can provide a PR if wanted.

    Please submit the PR if you want

    @Cito
    Copy link
    Mannequin Author

    Cito mannequin commented Apr 30, 2021

    I saw you submitted a PR already which looks good to me.

    @corona10
    Copy link
    Member

    I saw you submitted a PR already which looks good to me.

    I should care that you can have a chance to submit the patch :)
    Is it okay to proceed with this issue with the PR which I closed or do you want to open a PR with your hand?

    @corona10 corona10 removed 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Apr 30, 2021
    @Cito
    Copy link
    Mannequin Author

    Cito mannequin commented Apr 30, 2021

    I don't mind if you reopen your PR. But thanks for asking.

    @corona10
    Copy link
    Member

    I don't mind if you reopen your PR. But thanks for asking.

    I add your name as the co-author on PR 25756 :)
    Thanks for the report and suggestion

    @corona10 corona10 added the 3.10 only security fixes label Apr 30, 2021
    @corona10 corona10 added the 3.10 only security fixes label Apr 30, 2021
    @miss-islington
    Copy link
    Contributor

    New changeset 6143fcd by Dong-hee Na in branch 'master':
    bpo-43979: Remove unnecessary operation from urllib.parse.parse_qsl (GH-25756)
    6143fcd

    @corona10 corona10 closed this as completed May 1, 2021
    @corona10 corona10 closed this as completed May 1, 2021
    @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
    No one assigned
    Labels
    3.10 only security fixes 3.11 only security fixes performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @corona10 @miss-islington