Skip to content

Improve __all__ definitions for threading and _dummy_threading modules - #7135

Merged
JelleZijlstra merged 14 commits into
python:masterfrom
AlexWaygood:all
Feb 5, 2022
Merged

Improve __all__ definitions for threading and _dummy_threading modules#7135
JelleZijlstra merged 14 commits into
python:masterfrom
AlexWaygood:all

Conversation

@AlexWaygood

Copy link
Copy Markdown
Member

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@JelleZijlstra

Copy link
Copy Markdown
Member

Thanks, can you tell me what triggered the allowlist changes?

@AlexWaygood

Copy link
Copy Markdown
MemberAuthor

Thanks, can you tell me what triggered the allowlist changes?

There were a large number of "unused allowlist" complaints on 3.6-3.8 due to adding __all__ to _dummy_threading.pyi (I think these had previously not been imported by dummy_threading from _dummy_threading, due to the lack of __all__ in the stub). There were also several new "exists in the stub but not at runtime" complaints for 3.6-3.8 for the dummy_threading module -- again, I think this is because stubtest was simply ignoring these classes in the past, because they weren't being imported from _dummy_threading into dummy_threading.

Ideally we'd fix these problems (and I can do so in this PR if you like), but it felt out of scope for this PR.

Comment threadstdlib/threading.pyi
"stack_size",
"excepthook",
"ExceptHookArgs",
"gettrace",

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.

I don't see these last two in the code on 3.9.

I do see get_native_id, though it looks like it's absent on some OSes: https://docs.python.org/3.10/library/threading.html#threading.get_native_id

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.

Oops, this is a 3.10 block and gettrace and getprofile were only added in 3.10. Sorry for that.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Good catch on get_native_id. I was copy-pasting __all__ from the source code, and didn't spot that it was conditionally appended to __all__ lower down.

@AlexWaygoodAlexWaygoodFeb 5, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I have added get_native_id to threading.__all__, and I have deleted get_native_id from _dummy_threading.pyi, as stubtest correctly points out that it doesn't exist in the dummy_threading at runtime on Python 3.8.

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

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

@JelleZijlstra
JelleZijlstra merged commit 348c38a into python:masterFeb 5, 2022
@AlexWaygood
AlexWaygood deleted the all branch February 5, 2022 16:07
Sign up for freeto 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

@AlexWaygood@JelleZijlstra