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

stdlib: add defaults #9501

Merged
merged 22 commits into from
Jan 18, 2023
Merged

stdlib: add defaults #9501

merged 22 commits into from
Jan 18, 2023

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstra
Copy link
Member Author

To keep this PR manageable, I'm going to fix any issues in CI by just reverting the affected functions.

Notes on CI:

  • stubtest failure because stubtest doesn't see a str as compatible with a TypeVar(bound=str) in urllib.parse
  • urllib.parse.urlencode also has a default of None where our type is str, needs investigation; same for sysconfig.is_python_build, platform.libc_ver, ssl._create_unverified_context
  • and shutil.make_archive but with 0 vs. False; same for platform.platform, distutils.util.execute, distutils.util.byte_compile, distutils.spawn, and a bunch of others
  • flake8-pyi seems to think string defaults are annotations, so it will need a fix

@JelleZijlstra
Copy link
Member Author

We also got a pytype crash

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/pytype/pyi/visitor.py", line 36, in _call_visitor
    return super()._call_visitor(node)
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/pytype/ast/visitor.py", line 55, in _call_visitor
    return visitor(node)
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/pytype/pyi/parser.py", line [16](https://github.com/python/typeshed/actions/runs/3896137144/jobs/6652302058#step:7:17)2, in visit_Pyval
    return self.convert_late_annotation(node.value)
  File "/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/site-packages/pytype/pyi/parser.py", line 142, in convert_late_annotation
    typ = a.body[0].value  # pytype: disable=attribute-error
IndexError: list index out of range

I suspect like flake8-pyi it's something to do with treating string defaults as annotations but I'll have to look at the code.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@hauntsaninja
Copy link
Collaborator

Just checking, you ran latest stubtest locally with the default checking you added?

@JelleZijlstra
Copy link
Member Author

Not yet, I forgot it hadn't been released yet.

@JelleZijlstra JelleZijlstra marked this pull request as draft January 11, 2023 20:42
@hauntsaninja
Copy link
Collaborator

We've often used pre-release stubtest in CI. Might be a good idea to do that now too!

JelleZijlstra added a commit to JelleZijlstra/flake8-pyi that referenced this pull request Jan 12, 2023
JelleZijlstra added a commit to JelleZijlstra/pytype that referenced this pull request Jan 12, 2023
@JelleZijlstra
Copy link
Member Author

For reference fdc5863 is where we undid stubtest-from-master most recently, but some of the infra has changed since then so it's not just a matter of reverting and updating the commit.

@github-actions

This comment has been minimized.

JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request Jan 12, 2023
@github-actions

This comment has been minimized.

JelleZijlstra added a commit that referenced this pull request Jan 12, 2023
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

martindemello pushed a commit to google/pytype that referenced this pull request Jan 13, 2023
martindemello pushed a commit to google/pytype that referenced this pull request Jan 13, 2023
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@JelleZijlstra JelleZijlstra marked this pull request as ready for review January 18, 2023 06:36
@JelleZijlstra
Copy link
Member Author

New version with the latest stubdefaulter:

  • Now only adds defaults for 3.11 branches
  • Now also adds defaults to methods
  • Undid a bunch of things stubtest complained about

The PR currently doesn't include a change to the version of stubtest. If we want to run stubtest from mypy master, I'd rather do that in a separate PR.

@github-actions
Copy link
Contributor

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

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

A quick glance shows no obvious problems. But I mostly rely on our tests here.

@srittau srittau merged commit ddfaca3 into python:main Jan 18, 2023
AlexWaygood added a commit that referenced this pull request Jan 29, 2023
Continuing work towards #8988.

The first five commits were created using stubdefaulter on various Python versions; the following commits were all created manually by me to fix various problems. The main things this adds that weren't present in #9501 are:

- Defaults in Windows-only modules and Windows-only branches (because I'm running a Windows machine)
- Defaults in non-py311 branches
- Defaults for float parameters
- Defaults for overloads
Avasam pushed a commit to Avasam/typeshed that referenced this pull request Feb 1, 2023
Continuing work towards python#8988.

The first five commits were created using stubdefaulter on various Python versions; the following commits were all created manually by me to fix various problems. The main things this adds that weren't present in python#9501 are:

- Defaults in Windows-only modules and Windows-only branches (because I'm running a Windows machine)
- Defaults in non-py311 branches
- Defaults for float parameters
- Defaults for overloads
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.

4 participants