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

Jump to bottom

heapq: re-export, add __about__ #7308

Merged
merged 5 commits into from Feb 20, 2022
Merged

heapq: re-export, add __about__ #7308

merged 5 commits into from Feb 20, 2022

Conversation

hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Feb 20, 2022

No description provided.

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Feb 20, 2022

Looks like _heapq should be added to the list here:

typeshed/pyproject.toml

Line 12 in 879b617

extra_standard_library = [

@github-actions

This comment has been hidden.

Sign in to view
@hauntsaninja
Copy link
Collaborator Author

@hauntsaninja hauntsaninja commented Feb 20, 2022

Why does from _heapq import * lead to pyright import _T? import * doesn't import symbols that start with underscore. Or is there some special stub rule that I'm forgetting about.

@JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented Feb 20, 2022

Right, seems like a pyright bug.

@github-actions

This comment has been hidden.

Sign in to view
1 similar comment
@github-actions

This comment has been hidden.

Sign in to view
@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Feb 20, 2022

For now we can just rename one of the TypeVars though, right?

@hauntsaninja
Copy link
Collaborator Author

@hauntsaninja hauntsaninja commented Feb 20, 2022

It'd take me more time to rename the TypeVar than to tag @erictraut and have him make a pyright release :D

@erictraut
Copy link
Contributor

@erictraut erictraut commented Feb 20, 2022

I'm on vacation this week, so replies may be slower than normal.

Pyright's current behavior is:

  1. If a symbol is declared within a stub file and its name starts with an underscore, it is considered visible to other stubs or code files that import from it. For example, if stub.pyi includes the declaration _T = TypeVar("_T"), it is legal for a source file to do from stub import _T. In other words, names beginning with an underscore are visible outside of the stub.
  2. If someone performs a wildcard import from a stub and the stub contains a dunder all definition, only the symbols within the dunder all are imported.
  3. If someone performs a wildcard import from a stub and the stub does not contain a dunder all, all visible symbols (including those that begin with an underscore) are imported.

It sounds like you're suggesting that rule 3 is wrong? Is there a special-case exemption for wildcard imports from a stub file with regard to names that start with an underscore? If so, why? Is this special case documented somewhere?

@JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented Feb 20, 2022

This is not specific to stubs. If there is no __all__, import * excludes names that start with an underscore. This is documented in https://docs.python.org/3.10/reference/simple_stmts.html#the-import-statement (paragraph starting "The public names defined by a module").

@JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented Feb 20, 2022

(And enjoy your vacation! This bug can wait; it's easy to work around anyway.)

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Feb 20, 2022

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

Akuli
Akuli reviewed Feb 20, 2022
View changes
stdlib/heapq.pyi
def merge(*iterables: Iterable[_T], key: Callable[[_T], Any] | None = ..., reverse: bool = ...) -> Iterable[_T]: ...
def nlargest(n: int, iterable: Iterable[_T], key: Callable[[_T], SupportsRichComparison] | None = ...) -> list[_T]: ...
def nsmallest(n: int, iterable: Iterable[_T], key: Callable[[_T], SupportsRichComparison] | None = ...) -> list[_T]: ...
__about__: str
Copy link
Collaborator

@Akuli Akuli Feb 20, 2022

Choose a reason for hiding this comment

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

Is this needed? Usually __foo__ names are treated as public, not private like _foo names.

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Feb 20, 2022 •
edited

Choose a reason for hiding this comment

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

I think it is, or at least when I checked earlier it appears dunders are not imported using from x import * (also technically, in the impl it is defined, not exported)

Copy link
Collaborator

@Akuli Akuli Feb 20, 2022

Choose a reason for hiding this comment

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

Lol, cpython developers have decided to copy/paste the same text into both modules

>>> from _heapq import __about__ as a
>>> from heapq import __about__ as b
>>> a == b
True
>>> a is b
False

Akuli
Akuli approved these changes Feb 20, 2022
View changes
@Akuli Akuli merged commit 690864c into python:master Feb 20, 2022
61 checks passed
@hauntsaninja hauntsaninja deleted the heap branch Feb 20, 2022
@erictraut
Copy link
Contributor

@erictraut erictraut commented Feb 20, 2022

Thanks for the clarification. Here's a bug report to track the issue in pyright: microsoft/pyright#3087.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Akuli Akuli

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
5 participants
@hauntsaninja @AlexWaygood @JelleZijlstra @erictraut @Akuli