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-116126: Implement PEP 696 by JelleZijlstra · Pull Request #116129 · python/cpython

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented Feb 29, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This implements both the grammar and compiler changes and changes to typing.py behavior (the latter copied from typing-extensions).

  • Issue: Implement PEP 696 (Type parameter defaults) #116126

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Gobot1234, m-aciek, and ditansu reacted with heart emoji
All reactions
  • ❤️ 3 reactions
@JelleZijlstra
pythongh-116126: Grammar changes for PEP 696
e7ca092
@bedevere-app bedevere-app Bot mentioned this pull request Feb 29, 2024
Implement PEP 696 (Type parameter defaults) #116126
Closed
3 tasks
JelleZijlstra added 7 commits February 29, 2024 08:34
@JelleZijlstra
Fix test_ast
828679e
@JelleZijlstra
new tests
d3b472b
@JelleZijlstra
fix test_unparse
6e467ee
@JelleZijlstra
possible solution
fa48580
@JelleZijlstra
Can't use the name either
49b077b
@JelleZijlstra
blurb
5cfd5e2
@JelleZijlstra
should not have pushed that
01b3270
@JelleZijlstra

JelleZijlstra commented Mar 1, 2024

Copy link
Copy Markdown
Member Author

This should be ready for review now, I'll take it out of draft once the tests pass. Notes:

  • This PR intentionally does not cover documentation or typing.py behavior; I'll leave that to another PR.
  • The PEP 695 test suite helped me find some very interesting issues with the symtable effects of this change. I think the current solution should work, but definitely didn't expect that to be the trickiest aspect of the change.
  • It's unfortunate that the new AST field is called default_ in Python. It can't be default in C because that's a keyword. I may need to change the AST generation machinery to support aliasing a field so the Python-visible name can be just "default".
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra added 2 commits February 29, 2024 19:05
@JelleZijlstra
maybe this will compile
7a69204
@JelleZijlstra
Feature version support
a265008
@JelleZijlstra
JelleZijlstra marked this pull request as ready for review March 1, 2024 13:59
@JelleZijlstra
JelleZijlstra requested review from ericsnowcurrently, iritkatriel, isidentical, lysnikolaou, markshannon and pablogsal as code owners March 1, 2024 13:59
@bedevere-app bedevere-app Bot added the awaiting core review label Mar 1, 2024
@pablogsal

pablogsal commented Mar 1, 2024

Copy link
Copy Markdown
Member

Will review this weekend
JelleZijlstra reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JonathanPlasse JonathanPlasse mentioned this pull request Mar 11, 2024
Fix unimplemented default TypeVar argument in 3.13a4 HypothesisWorks/hypothesis#3917
Merged
@zsol zsol mentioned this pull request Apr 3, 2024
Implement PEP 696 (defaults for type params) Instagram/LibCST#1127
Closed
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into pep696v2
0c28297
@JelleZijlstra

JelleZijlstra commented Apr 10, 2024

Copy link
Copy Markdown
Member Author

@pablogsal have you had a chance to look at this? The grammar changes should be quite straightforward, so hopefully it's not complicated to review.

I'll spend some time today trying to make it so the field is called "default" not "default_" in Python, because having "default_" in the Python-visible APIs would be ugly.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra added 3 commits April 10, 2024 14:21
@JelleZijlstra
default_ -> default_value
8aa5b27 
Suggestion by Brandt.
@JelleZijlstra
ast docs
bdd01bb
@JelleZijlstra
more docs
2c28b9a
@JelleZijlstra
JelleZijlstra requested a review from AlexWaygood as a code owner April 10, 2024 18:48
AlexWaygood
AlexWaygood reviewed Apr 10, 2024
View reviewed changes
Comment thread Doc/library/typing.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 23, 2024
hauntsaninja
hauntsaninja reviewed Apr 23, 2024
View reviewed changes
Comment thread Doc/reference/executionmodel.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_unparse.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra and others added 5 commits April 23, 2024 14:19
@JelleZijlstra @hauntsaninja
Update Doc/reference/executionmodel.rst
9d4e842 
Co-authored-by: Shantanu <[email protected]>
@JelleZijlstra
Roundtrip tests
2686b97
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into pep696v2
98d1dec
@JelleZijlstra
Fix doctest
fbb6405
@JelleZijlstra
Add typing.NoDefault as the default for TypeVar's default
e0ccfeb
AlexWaygood
AlexWaygood reviewed Apr 28, 2024
View reviewed changes
Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/typing.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/typing.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/reference/compound_stmts.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra and others added 2 commits April 28, 2024 13:30
@JelleZijlstra @AlexWaygood
Apply suggestions from code review
15aaff9 
Co-authored-by: Alex Waygood <[email protected]>
@JelleZijlstra
Add .has_default() and update docs
86f5aed
AlexWaygood
AlexWaygood reviewed Apr 28, 2024
View reviewed changes

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

I would say "is not the typing.NoDefault singleton" rather than "is not equal to typing.NoDefault", since we usually argue that an identity check is more idiomatic for singleton objects such as None, NotImplemented and Ellipsis

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/typing.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/typing.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/typing.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood reviewed Apr 28, 2024
View reviewed changes
Comment thread Doc/library/typing.rst
Comment on lines +3244 to +3248
>>> T.__default__ is typing.NoDefault
True
>>> S = TypeVar("S", default=None)
>>> S.__default__ is None
True

@AlexWaygood AlexWaygood Apr 28, 2024

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.

Suggested change
>>> T.__default__ is typing.NoDefault
True
>>> S = TypeVar("S", default=None)
>>> S.__default__ is None
True
>>> T.has_default()
False
>>> T.__default__ is typing.NoDefault
True
>>> S = TypeVar("S", default=None)
>>> S.has_default()
True
>>> S.__default__ is None
True

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra JelleZijlstra Apr 28, 2024

Copy link
Copy Markdown
Member Author

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.

I thought of that but it felt out of place in the docs for NoDefault, since the cases you added don't use NoDefault at all.

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood AlexWaygood Apr 28, 2024

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.

Fair enough -- I thought it was quite nice to see together in one example how the two concepts interrelate, but I definitely don't feel strongly!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
JelleZijlstra and others added 2 commits April 28, 2024 14:29
@JelleZijlstra @AlexWaygood
Apply suggestions from code review
3e0c0fc 
Co-authored-by: Alex Waygood <[email protected]>
@JelleZijlstra
remove trailing space
f5a3d4b
@JelleZijlstra

JelleZijlstra commented Apr 28, 2024

Copy link
Copy Markdown
Member Author

@ericsnowcurrently the C globals check is failing because this PR now adds a new static type and global singleton, typing.NoDefault (closely analogous to builtins.NotImplemented). What do you think we should do? We could simply allowlist the new globals, or somehow make NoDefault not rely on C globals—but the latter feels like it would be a lot more complicated.
All reactions

Sorry, something went wrong.

Uh oh!

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

ericsnowcurrently commented Apr 30, 2024

Copy link
Copy Markdown
Member

For now let's just whitelist the new singleton, as long as it is stateless and immortal. We can circle back later if the whitelist is a problem.
JelleZijlstra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JelleZijlstra added 2 commits April 29, 2024 17:59
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into pep696v2
92ea108
@JelleZijlstra
Ignore new C globals
5f6fdfd
@JelleZijlstra JelleZijlstra added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 30, 2024
@bedevere-bot

bedevere-bot commented Apr 30, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit 5f6fdfd 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 30, 2024
JelleZijlstra added 2 commits May 2, 2024 18:50
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into pep696v2
b3f053c
@JelleZijlstra
Fix scoping key
8c3e0b4 
The previous logic would add the size of the pointer target to the pointer value, which might point to another pointer being used as a scope key. Now, we increment the value as an integer instead, which means it can never be a valid pointer.
@JelleZijlstra
JelleZijlstra merged commit ca269e5 into python:main May 3, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label May 3, 2024
@JelleZijlstra
JelleZijlstra deleted the pep696v2 branch May 3, 2024 13:17
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@JelleZijlstra @AlexWaygood
@picnixz @hauntsaninja @SonicField
pythongh-116126: Implement PEP 696 (python#116129)
5b81f70 
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Shantanu <[email protected]>
@JelleZijlstra JelleZijlstra mentioned this pull request May 13, 2024
gh-98306: Support JSON encoding of NaNs and infinities as null #115246
Closed
@JelleZijlstra
JelleZijlstra restored the pep696v2 branch September 10, 2024 23:37
@JelleZijlstra
JelleZijlstra deleted the pep696v2 branch September 24, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm left review comments
@picnixz picnixz picnixz left review comments
@hauntsaninja hauntsaninja hauntsaninja left review comments
@erlend-aasland erlend-aasland erlend-aasland left review comments
@AlexWaygood AlexWaygood AlexWaygood approved these changes
@isidentical isidentical Awaiting requested review from isidentical isidentical is a code owner
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

9 participants

@JelleZijlstra @pablogsal @AlexWaygood @bedevere-bot @ericsnowcurrently @carljm @picnixz @hauntsaninja @erlend-aasland
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.