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 — [3.12] gh-104799: Move location of type_params AST fields (GH-104828) by miss-islington · Pull Request #104974 · python/cpython

Conversation

@miss-islington

@miss-islington miss-islington commented May 26, 2023 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

(cherry picked from commit ba73473)

Co-authored-by: Jelle Zijlstra [email protected]
Co-authored-by: Alex Waygood [email protected]

  • Issue: PEP-695: Potentially breaking changes made to __match_args__ attributes of AST nodes #104799

📚 Documentation preview 📚: https://cpython-previews--104974.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

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

All reactions
@JelleZijlstra @AlexWaygood @miss-islington
pythongh-104799: Move location of type_params AST fields (pythonGH-10…
4e5484e
…4828)

(cherry picked from commit ba73473)

Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
@miss-islington
miss-islington requested review from lysnikolaou and pablogsal as code owners May 26, 2023 12:54
@bedevere-bot bedevere-bot added the awaiting review label May 26, 2023
This was referenced May 26, 2023
PEP-695: Potentially breaking changes made to __match_args__ attributes of AST nodes #104799
Closed
gh-104799: Move location of type_params AST fields #104828
Merged
@JelleZijlstra JelleZijlstra self-assigned this May 26, 2023
@JelleZijlstra
JelleZijlstra enabled auto-merge (squash) May 26, 2023 12:56
@JelleZijlstra

JelleZijlstra commented May 26, 2023

Copy link
Copy Markdown
Member

Seems like this changes the ABI! cc @Yhg1s

I don't know whether the ABI change is relevant in practice, because external users shouldn't be constructing these internal AST nodes anyway. So maybe we should decide this is a false positive. If we think this is in fact a real issue that could affect users, I would vote to revert the change on main too and leave #104799 unfixed, as it's a relatively minor issue and the AST doesn't guarantee full backwards compatibility anyway.

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

@AlexWaygood AlexWaygood mentioned this pull request May 27, 2023
gh-103921: Document PEP 695 #104642
Merged
@AlexWaygood

AlexWaygood commented May 30, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

@Yhg1s, thoughts on this, as release manager? My preference would be to get this merged into 3.12 ASAP, as it minimizes the backwards-compatibility breakage between 3.11 and 3.12 that comes from the PEP-695 implementation. But if the failing ABI check makes that impossible, then we should revert the change that's been made on main (which this PR is backporting) ASAP.

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood
AlexWaygood requested a review from Yhg1s May 30, 2023 10:19
@JelleZijlstra

JelleZijlstra commented May 30, 2023 •
edited by AlexWaygood
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

@Yhg1s said on Discord that the ABI change is fine (thanks!). I'm running the script to regenerate the ABI locally now.

All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented May 30, 2023

Copy link
Copy Markdown
Member

Unfortunately I couldn't get the script to work. On Mac something is wrong with libmpdec

gcc -I./Modules/_decimal/libmpdec -DCONFIG_64=1 -DANSI=1 -DHAVE_UINT128_T=1 -fno-strict-overflow -DNDEBUG -g -O3 -Wall -g3 -O0 -g3 -O0  -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal  -I. -I./Include   -fPIC -fPIC -c ./Modules/_decimal/_decimal.c -o Modules/_decimal/_decimal.o
gcc -shared      Modules/_decimal/_decimal.o -lm Modules/_decimal/libmpdec/libmpdec.a  -o Modules/_decimal.cpython-312-aarch64-linux-gnu.so
/usr/bin/ld: Modules/_decimal/_decimal.o: in function `context_getprec':
/src/./Modules/_decimal/_decimal.c:740: undefined reference to `mpd_getprec'
/usr/bin/ld: Modules/_decimal/_decimal.o: in function `context_getemax':
/src/./Modules/_decimal/_decimal.c:741: undefined reference to `mpd_getemax'

On an AWS Ubuntu instance I get error="docker-credential-ecr-login can only be used with Amazon Elastic Container Registry.".

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood
Regenerate the ABI
170f3d3
@AlexWaygood

AlexWaygood commented May 30, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

I used the workflow Steve is demo-ing over at #105088 to regen the ABI via GitHub Actions on my CPython fork. I think I did it right. At least the GitHub Actions check on this PR is now passing!

zooba reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented May 30, 2023

Copy link
Copy Markdown
Member

Thanks! That diff looks pretty reasonable.

AlexWaygood reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra
JelleZijlstra merged commit 7899fac into python:3.12 May 30, 2023
@bedevere-bot bedevere-bot removed the awaiting review label May 30, 2023
@miss-islington
miss-islington deleted the backport-ba73473-3.12 branch May 30, 2023 14:33
This was referenced May 30, 2023
gh-103921: Minor PEP-695 fixes to the ast module docs #105093
Merged
Update ast for 3.12 python/typeshed#10201
Merged
@erlend-aasland erlend-aasland mentioned this pull request Jun 6, 2023
3.12 backport gh 105236 #105358
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 17, 2026
Add script/code block action to FlowDefinition crewAIInc/crewAI#6197
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@Yhg1s Yhg1s Awaiting requested review from Yhg1s

Uh oh!

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

Assignees

@JelleZijlstra JelleZijlstra

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.

4 participants

@miss-islington @JelleZijlstra @AlexWaygood @bedevere-bot
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.