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

resort _ast and ast modules #11162

Merged
merged 9 commits into from
Oct 5, 2024
Merged

resort _ast and ast modules #11162

merged 9 commits into from
Oct 5, 2024

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Dec 14, 2023

Well, I got a lukewarm to positive reaction from #11141, about what to do when c-implemented classes claim their module is the non-private one. So here's an MR to re-organize ast and _ast and get the type stubs to match what the implementation claims.

Mostly this is just lift and shift, but I took the opportunity to make the order within the file more closely match the implementation - that seems to be preferred?

related to #3968 and the discussion in #11141

The classes imported from the _ast module are defined in C,
but set their __module__ to ast. This arrangement ensures that
the type stubs have the same.

related to python#3968
and the discussion in python#11141
@tungol tungol marked this pull request as draft December 14, 2023 02:47

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@tungol tungol marked this pull request as ready for review December 14, 2023 03:20
@tungol tungol marked this pull request as draft December 16, 2023 23:27
@JelleZijlstra
Copy link
Member

This is heavily merge conflicted now and it's probably better to restart from scratch. I merged a few similar PRs recently and would be happy to review and merge this one if the conflicts are resolved.

3.8 is also now almost at EOL, so it's less important if we get the module wrong in 3.8.

Copy link
Contributor

github-actions bot commented Oct 4, 2024

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@tungol tungol marked this pull request as ready for review October 4, 2024 20:02
@tungol
Copy link
Contributor Author

tungol commented Oct 4, 2024

This MR should be ready now as well.

@JelleZijlstra JelleZijlstra self-requested a review October 4, 2024 20:53
@JelleZijlstra JelleZijlstra merged commit 77510ae into python:main Oct 5, 2024
63 checks passed
@tungol tungol deleted the ast branch October 5, 2024 01:05
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.

2 participants