GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-100746: Improve `test_named_expressions.py` by sobolevn · Pull Request #116713 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Mar 13, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Two major changes:

  1. I removed pass where possible and instead used an assert for what we expect to happen
  2. I added dict-comprehensions testing by copy-pasting and modifing set-comprehension tests. Why? Because dict has key and value, we need to be sure that both work correctly.
  • Issue: Dict comprehensions are not tested at all in test_named_expressions.py #100746

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
pythongh-100746: Improve test_named_expressions.py
b1562be
@sobolevn
sobolevn requested review from emilyemorehouse and pablogsal March 13, 2024 08:03
@bedevere-app bedevere-app Bot added the awaiting core review label Mar 13, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Mar 13, 2024
Dict comprehensions are not tested at all in test_named_expressions.py #100746
Closed
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Mar 13, 2024
@sobolevn sobolevn added the skip news label Mar 13, 2024
emilyemorehouse
emilyemorehouse approved these changes Mar 13, 2024
View reviewed changes

@emilyemorehouse emilyemorehouse left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks great, thanks for getting more test coverage on this!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sobolevn reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Mar 13, 2024
@sobolevn sobolevn added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Mar 13, 2024
@sobolevn

sobolevn commented Mar 13, 2024

Copy link
Copy Markdown
Member Author

Backport to 3.12 will be as-is, while 3.11 will be much smaller, because 3.11 does not have new binding logic from 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Eclips4
Eclips4 approved these changes Mar 13, 2024
View reviewed changes

@Eclips4 Eclips4 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
sobolevn merged commit 25684e7 into python:main Mar 13, 2024
@miss-islington-app

miss-islington-app Bot commented Mar 13, 2024

Copy link
Copy Markdown

Thanks @sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 13, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 13, 2024
@sobolevn @miss-islington
pythongh-100746: Improve test_named_expressions.py (pythonGH-116713)
0b2e876 
(cherry picked from commit 25684e7)

Co-authored-by: Nikita Sobolev <[email protected]>
@bedevere-app

bedevere-app Bot commented Mar 13, 2024

Copy link
Copy Markdown

GH-116747 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Mar 13, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 13, 2024
@sobolevn @miss-islington
pythongh-100746: Improve test_named_expressions.py (pythonGH-116713)
4e72e66 
(cherry picked from commit 25684e7)

Co-authored-by: Nikita Sobolev <[email protected]>
@bedevere-app

bedevere-app Bot commented Mar 13, 2024

Copy link
Copy Markdown

GH-116748 is a backport of this pull request to the 3.11 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Mar 13, 2024
sobolevn added a commit that referenced this pull request Mar 13, 2024
@miss-islington @sobolevn
[3.12] gh-100746: Improve test_named_expressions.py (GH-116713) (#1…
734c5c4 
…16747)

gh-100746: Improve `test_named_expressions.py` (GH-116713)
(cherry picked from commit 25684e7)

Co-authored-by: Nikita Sobolev <[email protected]>
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
@sobolevn @vstinner
pythongh-100746: Improve test_named_expressions.py (python#116713)
41f5bc2
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@sobolevn @adorilson
pythongh-100746: Improve test_named_expressions.py (python#116713)
69861cd
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@sobolevn @diegorusso
pythongh-100746: Improve test_named_expressions.py (python#116713)
1e73fce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emilyemorehouse emilyemorehouse emilyemorehouse approved these changes
@Eclips4 Eclips4 Eclips4 approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sobolevn @emilyemorehouse @Eclips4
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.