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

bpo-28837: Fix lib2to3 handling of map/zip/filter #24

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

stuarteberg
Copy link
Contributor

@stuarteberg stuarteberg commented Feb 11, 2017

The 2to3 tool will automatically wrap calls to map, zip, and filter with list():

# Original:
zip('abc', '123')

# After 2to3:
list(zip('abc', '123'))

...but the tool misses some cases, if the call is followed by a "trailer" such as [0]:

# Skipped by 2to3:
zip('abc', '123')[0]

This PR augments the lib2to3 "fixers" for map, zip, and filter to correctly handle such cases.

http://bugs.python.org/issue28837

@codecov
Copy link

codecov bot commented Feb 11, 2017

Codecov Report

Merging #24 into master will increase coverage by <.01%.

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
+ Coverage   82.37%   82.37%   +<.01%     
==========================================
  Files        1427     1427              
  Lines      350948   351009      +61     
==========================================
+ Hits       289088   289144      +56     
- Misses      61860    61865       +5

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 e7ffb99...5cdd2db. Read the comment docs.

@stuarteberg
Copy link
Contributor Author

@benjaminp: Are you the right person to ping about lib2to3 PRs?

>
|
power<
'map' trailer< '(' [arglist=any] ')' >
'map' args=trailer< '(' [any] ')' >
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the reason for this change?

On line 100 (below), I need to clone the argument list:

new = Node(syms.power, [Name("map"), args.clone()])

If I leave the above line unchanged, then I could have written this, instead:

new = Node(syms.power, [Name("map"), ArgList([args.clone()])])

...but there's a problem with that: It doesn't clone the ( and ) tokens -- it creates them from scratch. As a result, one of the tests fails, due to an unpreserved prefix before the ) token:

======================================================================
FAIL: test_prefix_preservation (Lib.lib2to3.tests.test_fixers.Test_map)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/bergs/Documents/workspace/cpython-git/Lib/lib2to3/tests/test_fixers.py", line 3036, in test_prefix_preservation
    self.check(b, a)
  File "/Users/bergs/Documents/workspace/cpython-git/Lib/lib2to3/tests/test_fixers.py", line 3031, in check
    super(Test_map, self).check(b, a)
  File "/Users/bergs/Documents/workspace/cpython-git/Lib/lib2to3/tests/test_fixers.py", line 36, in check
    tree = self._check(before, after)
  File "/Users/bergs/Documents/workspace/cpython-git/Lib/lib2to3/tests/test_fixers.py", line 32, in _check
    self.assertEqual(after, str(tree))
AssertionError: "x =    list(map(   f,    'abc'   ))\n\n" != "x =    list(map(   f,    'abc'))\n\n"
- x =    list(map(   f,    'abc'   ))
?                               ---
+ x =    list(map(   f,    'abc'))

Hence, I think it's best to capture and clone the whole trailer. But if I'm missing something, please let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, thanks for the explanation.

@benjaminp benjaminp merged commit 93b4b47 into python:master Apr 6, 2017
Mariatta referenced this pull request in Mariatta/cpython Jun 16, 2017
…llowed with a 'trailer', e.g. zip()[x] (GH-24)

(cherry picked from commit 93b4b47)
Mariatta added a commit that referenced this pull request Jun 16, 2017
… with a 'trailer', e.g. zip()[x] (GH-24) (GH-2235)

(cherry picked from commit 93b4b47)
jaraco pushed a commit that referenced this pull request Dec 2, 2022
jaraco pushed a commit to jaraco/cpython that referenced this pull request Feb 17, 2023
Refactor into Python 3 and Python 2.7 submodules.
nanjekyejoannah added a commit to nanjekyejoannah/cpython that referenced this pull request Mar 10, 2023
24: Warn for exception: move warning to ceval r=ltratt a=nanjekyejoannah

This replaces the old PR: softdevteam#12
Moved the warning to ceval.

I removed the three component warning because it was committed in an earlier PR here: softdevteam#14

Co-authored-by: Joannah Nanjekye <jnanjeky@unb.ca>
Fidget-Spinner referenced this pull request in pylbbv/pylbbv May 27, 2023
oraluben pushed a commit to oraluben/cpython that referenced this pull request Jun 25, 2023
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