Skip to content

Fix types in _interpqueues and _interpreters #13355

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

Merged
merged 5 commits into from
Jan 2, 2025

Conversation

max-muoto
Copy link
Contributor

Fix several types in both modules as described in #13351, additionally I caught some other issues:

  • _interpqueues.list_all() now returns the unboundop as the third item in each tuple.
  • _interpqueues.get_queue_defaults() no returns the unboundop as the second item in each tuple.

This comment has been minimized.

@@ -21,7 +21,9 @@ def get_main() -> tuple[int, int]: ...
def is_running(id: SupportsIndex, *, restrict: bool = False) -> bool: ...
def get_config(id: SupportsIndex, *, restrict: bool = False) -> types.SimpleNamespace: ...
def whence(id: SupportsIndex) -> int: ...
def exec(id: SupportsIndex, code: str, shared: bool | None = None, *, restrict: bool = False) -> None: ...
def exec(
id: SupportsIndex, code: str | types.CodeType, shared: bool | None = None, *, restrict: bool = False
Copy link
Contributor

Choose a reason for hiding this comment

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

It also supports (argument-less) free functions, as per its documentation, and I've verified this in practice. I'm not sure there's any way to enforce these restrictions via the type annotations though.

Suggested change
id: SupportsIndex, code: str | types.CodeType, shared: bool | None = None, *, restrict: bool = False
id: SupportsIndex, code: str | types.CodeType | Callable[[], Any], shared: bool | None = None, *, restrict: bool = False

Copy link
Contributor Author

@max-muoto max-muoto Jan 2, 2025

Choose a reason for hiding this comment

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

Was able to verify as such, will add this but use object as the return type to stay consistent with the other types here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from typing import Any, Literal, SupportsIndex
from typing_extensions import TypeAlias

_UnboundOp: TypeAlias = Literal[1, 2, 3]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific reason for this, rather than using SupportsIndex? The way I see it, I can't pass my own constants like UNBOUND = 2; I'd have to pass a literal 2 to each call.

Copy link
Contributor Author

@max-muoto max-muoto Jan 2, 2025

Choose a reason for hiding this comment

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

That's the general pattern across typeshed when literals are required to avoid a runtime error, additionally, what you're describing would work if you're using Pyright: https://pyright-play.net/?pyrightVersion=1.1.384&pythonVersion=3.14&strict=true&code=GYJw9gtgBALgngBwJYDsDmUkQWEMoAySMApiAIYA2AUNQLICaA%2BgQJIAqUAvFAEy0ATEsCjAwYABQAPAFyFiZKgG1eAXQCUUALQA%2BKADkwKEnIB052tTGTGLDuupA

If you're using MyPy, my recommendation would be to annotate UNBOUND with Final, which will treat it as an immutable constant and automatically infer the literal: https://mypy-play.net/?mypy=latest&python=3.12&gist=6f3c5ecd5b1a4dba41ac352a57694898

Copy link
Contributor

github-actions bot commented Jan 2, 2025

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

agronholm added a commit to agronholm/anyio that referenced this pull request Jan 2, 2025
@JelleZijlstra JelleZijlstra merged commit 88e917a into python:main Jan 2, 2025
64 checks passed
hoel-bagard pushed a commit to hoel-bagard/typeshed that referenced this pull request Jan 5, 2025
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.

3 participants