Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b.#18477

Merged
serhiy-storchaka merged 2 commits into
python:masterpython/cpython:masterfrom
serhiy-storchaka:ast-start-pos-trailersserhiy-storchaka/cpython:ast-start-pos-trailersCopy head branch name to clipboard
Feb 12, 2020
Merged

bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b.#18477
serhiy-storchaka merged 2 commits into
python:masterpython/cpython:masterfrom
serhiy-storchaka:ast-start-pos-trailersserhiy-storchaka/cpython:ast-start-pos-trailersCopy head branch name to clipboard

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Feb 12, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.8 labels Feb 12, 2020
@codecov

codecov Bot commented Feb 12, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #18477     +/-   ##
=========================================
  Coverage   82.11%   82.12%             
=========================================
  Files        1956     1955      -1     
  Lines      589067   583729   -5338     
  Branches    44436    44437      +1     
=========================================
- Hits       483723   479376   -4347     
+ Misses      95690    94708    -982     
+ Partials     9654     9645      -9
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 327 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 95905ce...f0663f0. Read the comment docs.

@pablogsal pablogsal mentioned this pull request Feb 12, 2020
Merged
6 tasks
lysnikolaou
lysnikolaou approved these changes Feb 12, 2020
View reviewed changes

@lysnikolaou lysnikolaou 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!

gvanrossum
gvanrossum approved these changes Feb 12, 2020
View reviewed changes

@gvanrossum gvanrossum 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.

From a quick glance LG.

Would it make sense to add a test for e.g. ((a)).b?

@serhiy-storchaka

serhiy-storchaka commented Feb 12, 2020

Copy link
Copy Markdown
Member Author

We can add complex examples like ((((a+b).c)[d])(e))().f, but it would not cover anything that was not already covered by the minimal example (a).b.

@gvanrossum

gvanrossum commented Feb 12, 2020

Copy link
Copy Markdown
Member

I don't need complex example, but I'd like a single example showing ((a)).b, since IIRC there's some special code involved in removing redundant parentheses, and I feel that that special case was involved in the bug in the first place.

@gvanrossum

gvanrossum commented Feb 12, 2020

Copy link
Copy Markdown
Member

Honestly I take it back, you can land.

@serhiy-storchaka serhiy-storchaka merged commit 6e619c4 into python:master Feb 12, 2020
@miss-islington

miss-islington commented Feb 12, 2020

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the ast-start-pos-trailers branch February 12, 2020 20:37
@bedevere-bot

bedevere-bot commented Feb 12, 2020

Copy link
Copy Markdown

GH-18491 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Feb 12, 2020
@miss-islington @serhiy-storchaka
bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b. (…
2076d4f 
…GH-18477)

(cherry picked from commit 6e619c4)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@sthagen sthagen mentioned this pull request Feb 12, 2020
Merged
@lysnikolaou lysnikolaou mentioned this pull request Feb 13, 2020
Closed
colesbury referenced this pull request in colesbury/nogil Oct 6, 2021
@serhiy-storchaka @colesbury
bpo-39474: Fix AST pos for expressions like (a)(b), (a)[b] and (a).b.…
90a5109 
… (GH-18477)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@lysnikolaou lysnikolaou lysnikolaou approved these changes
@isidentical isidentical isidentical approved these changes

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@serhiy-storchaka @gvanrossum @miss-islington @bedevere-bot @lysnikolaou @isidentical @the-knights-who-say-ni
Morty Proxy This is a proxified and sanitized view of the page, visit original site.