Skip to content

Improve sys stubs #6816

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 13 commits into from
Jan 5, 2022
Merged

Improve sys stubs #6816

merged 13 commits into from
Jan 5, 2022

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jan 4, 2022

There are quite a few errors in the stubs for the sys module. Most of these appear to have slipped through the net due to the fact that stubtest isn't much good for this module.

For example, take the sys.flags object. In typeshed, we type it like this:

class _flags:
    # rest of the class definition here

flags: _flags

But at runtime, it actually looks like this:

>>> import sys
>>> sys.flags
sys.flags(debug=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=0, no_site=0, ignore_environment=0, verbose=0, bytes_warning=0, quiet=0, hash_randomization=1, isolated=0, dev_mode=False, utf8_mode=0)
>>> type(sys.flags)
<class 'sys.flags'>

Yup, that's a variable that has the same name as the class it's an instance of. As a result of this sys-module magic, stubtest just hasn't been checking the following documented attributes in sys:

  • sys.flags
  • sys.version_info
  • sys.float_info
  • sys.hash_info
  • sys.int_info
  • The return type of sys.getwindowsversion()
  • The return type of sys.get_asyncgen_hooks()

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

stdlib/sys.pyi Outdated
Comment on lines 302 to 326
if sys.platform == "win32":
# A tuple of length 5, even though it has more than 5 attributes.
@final
class _WinVersion(_uninstantiable_structseq, tuple[int, int, int, int, str]):
@property
def major(self) -> int: ...
@property
def minor(self) -> int: ...
@property
def build(self) -> int: ...
@property
def platform(self) -> int: ...
@property
def service_pack(self) -> str: ...
@property
def service_pack_minor(self) -> int: ...
@property
def service_pack_major(self) -> int: ...
@property
def suite_mast(self) -> int: ...
@property
def product_type(self) -> int: ...
@property
def platform_version(self) -> tuple[int, int, int]: ...
def getwindowsversion() -> _WinVersion: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note to self: I will have to spin up a windows virtual machine to check these. Will mark resolved when done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: suite_mast --> suite_mask

Copy link
Member Author

@AlexWaygood AlexWaygood Jan 4, 2022

Choose a reason for hiding this comment

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

I have fixed the typo (it predated this PR, FWIW)

Co-authored-by: Akuli <akuviljanen17@gmail.com>
@github-actions

This comment has been minimized.

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

1 similar comment
@github-actions

This comment has been minimized.

AlexWaygood added a commit to AlexWaygood/typeshed that referenced this pull request Jan 5, 2022
As discussed in python#6816, for some `structseq` classes these attributes are writeable `ClassVar`s, but for other classes, they're read-only `ClassVar`s. Either way, however, it's probably a bad idea to be modifying these attributes, so it makes sense to simply annotate them with `Final` rather than `ClassVar`.

Note that [PEP 591](https://www.python.org/dev/peps/pep-0591/) specifically states:

> Type checkers should infer a final attribute that is initialized in a class body as being a class variable. Variables should not be annotated with both ClassVar and Final.

As such, annotating these attributes as `Final[ClassVar[int]]` would be neither necessary nor possible.
Copy link
Collaborator

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

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

@Akuli Akuli merged commit beedc1d into python:master Jan 5, 2022
@AlexWaygood AlexWaygood deleted the sys-structseqs branch January 5, 2022 15:38
@Akuli Akuli mentioned this pull request Jan 5, 2022
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