bpo-38641: Add support of starred expressions in return/yield to lib2to3 by vemel · Pull Request #16994 · python/cpython · GitHub
Skip to content

bpo-38641: Add support of starred expressions in return/yield to lib2to3 - #16994

Merged
pablogsal merged 9 commits into
python:masterfrom
vemel:bpo-32117-lib2to3
Mar 1, 2020
Merged

bpo-38641: Add support of starred expressions in return/yield to lib2to3#16994
pablogsal merged 9 commits into
python:masterfrom
vemel:bpo-32117-lib2to3

Conversation

@vemel

@vemel vemel commented Oct 30, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Goal

bpo-32117 is fixed, but unfortunately lib2to3 still does not support unpacking in return and yield without parentheses.

def test():
    my_list = ["value2", "value3"]
    yield "value1", *my_list
    return "value1", *my_list

Changes

  • Use testlist_star_expr in return_stmt
  • Use testlist_star_expr in yield_arg

Summary

This is a backwards compatible change, because testlist_star_expr supports test as well like testlist does.

https://bugs.python.org/issue38641

@the-knights-who-say-ni

the-knights-who-say-ni commented Oct 30, 2019

Copy link
Copy Markdown

@vemel vemel changed the title bpo-32117: Add support to lib2to3 bpo-38641: Add support of starred expressions in return/yield to lib2to3 Oct 30, 2019
@vemel vemel mentioned this pull request Oct 30, 2019
Closed
@pablogsal

pablogsal commented Oct 30, 2019

Copy link
Copy Markdown
Member

Hi @vemel and thanks for the change!

Could you sign the CLA and add a NEWS entry? Thanks!

@vemel

vemel commented Oct 30, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi @vemel and thanks for the change!

Could you sign the CLA and add a NEWS entry? Thanks!

I have already signed CLA. Going to add news section now.
I also adapted grammar unit tests

@vemel

vemel commented Oct 30, 2019

Copy link
Copy Markdown
Contributor Author

Just added news section.

isidentical
isidentical reviewed Feb 16, 2020
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2019-10-30-15-31-09.bpo-38641.HrTL9k.rst Outdated
@@ -0,0 +1 @@
Added starred expressions support to ``return`` and ``yield`` statements.

@isidentical isidentical Feb 16, 2020

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.

Can you specify this is a bug related to lib2to3 here, also you can add a Patch by <name>. attribution at the end of news.

@vemel vemel Feb 17, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated, thank you!

Comment thread Misc/ACKS
Comment thread Lib/lib2to3/tests/data/py3_test_grammar.py
@vemel
vemel requested a review from isidentical February 17, 2020 21:16
@codecov

codecov Bot commented Feb 17, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #16994 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #16994     +/-   ##
=========================================
  Coverage   82.11%   82.12%             
=========================================
  Files        1956     1955      -1     
  Lines      589364   584024   -5340     
  Branches    44457    44457             
=========================================
- Hits       483966   479605   -4361     
+ Misses      95746    94772    -974     
+ Partials     9652     9647      -5
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 325 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c1b6a6...90405a3. Read the comment docs.

@vemel

vemel commented Feb 17, 2020

Copy link
Copy Markdown
Contributor Author

Looks like unit test for MacOS are broken, but it is not related to current PR. Probably it makes sense to fix them in another PR.

isidentical
isidentical approved these changes Feb 19, 2020
View reviewed changes

@isidentical isidentical left a comment •
edited
Loading

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.

LGTM except for the ACKS file, I think you did something wrong while you were rebasing/merging. Can you check it again @vemel

@vemel

vemel commented Feb 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

LGTM except for the ACKS file, I think you did something wrong while you were rebasing/merging. Can you check it again @vemel

Indeed. Fixed.

@vemel
vemel requested a review from isidentical February 20, 2020 17:51
pablogsal
pablogsal approved these changes Mar 1, 2020
View reviewed changes

@pablogsal pablogsal left a comment

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.

LGTM

Thanks, @vemel for the PR and @isidentical for the review :)

@pablogsal
pablogsal merged commit 768d739 into python:master Mar 1, 2020
@isidentical

isidentical commented Mar 1, 2020

Copy link
Copy Markdown
Member

@sthagen sthagen mentioned this pull request Mar 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes
@isidentical isidentical isidentical approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vemel @the-knights-who-say-ni @pablogsal @isidentical @bedevere-bot