GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-104050: Run mypy on `clinic.py` in CI by AlexWaygood · Pull Request #104421 · python/cpython

Conversation

@AlexWaygood

@AlexWaygood AlexWaygood commented May 12, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Add a basic mypy check to clinic.py in CI (and add some initial type annotations, so that the mypy check passes).

The check will run:

  • On pushes to main
  • On pull requests that touch files in the Tools/clinic directory, and pull requests that touch the workflow file itself
  • When contributors manually request a workflow on their fork, via workflow_dispatch.

When it does run, mypy will only check files inside Tools/clinic/. All other files will be excluded from the mypy check in CI.

The mypy configuration is also deliberately lax, to begin with: only a few of the stricter settings are enabled. This will allow us to make use of "gradual typing": mypy will only check functions inside Tools/clinic/ that we've added annotations to. It will assume all other functions are to be ignored by mypy, for now; we will therefore be able to gradually increase the level of type annotations inside this directory.

  • Issue: Add type annotations to clinic.py #104050

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
erlend-aasland and others added 6 commits May 1, 2023 18:37
@erlend-aasland
Slightly annotate cpp.py
ca8532b
@erlend-aasland
Slightly annotate clinic.py
d2ec2ef
@erlend-aasland
Address review
801af38 
- ditch no-return
- use callable from collections.abc
- use namedtuple from typing
@AlexWaygood
Add basic mypy workflow to CI
00183ff
@AlexWaygood
Make the type check pass
4bdd409
@AlexWaygood
Tweak workflow file
632843b
@AlexWaygood
AlexWaygood requested a review from erlend-aasland May 12, 2023 12:38
@AlexWaygood
AlexWaygood requested review from ezio-melotti and hugovk as code owners May 12, 2023 12:39
@bedevere-bot bedevere-bot added the awaiting core review label May 12, 2023
@bedevere-bot bedevere-bot mentioned this pull request May 12, 2023
Add type annotations to clinic.py #104050
Closed
@AlexWaygood AlexWaygood added the skip news label May 12, 2023
AlexWaygood
AlexWaygood commented May 12, 2023
View reviewed changes
Comment thread Tools/clinic/mypy.ini

@AlexWaygood AlexWaygood May 12, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I usually put my mypy config in a pyproject.toml file these days, but a mypy.ini file feels like it makes more "sense" unless we're planning on adding other linters/formatters to check clinic.py in the future

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
AlexWaygood
AlexWaygood commented May 12, 2023
View reviewed changes
Comment thread .github/workflows/mypy-clinic.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
improve workflow file
f7e7a25
JelleZijlstra
JelleZijlstra reviewed May 12, 2023
View reviewed changes
Comment thread Tools/clinic/clinic.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sobolevn
sobolevn reviewed May 12, 2023
View reviewed changes
Comment thread .github/workflows/mypy-clinic.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/clinic.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/clinic.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/cpp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/cpp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/cpp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
hugovk
hugovk reviewed May 12, 2023
View reviewed changes
Comment thread .github/workflows/mypy-clinic.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread .github/workflows/mypy-clinic.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/clinic.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread .github/workflows/mypy-clinic.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood and others added 3 commits May 12, 2023 14:18
@AlexWaygood @sobolevn
Add some more return annotations
510e08b 
Co-authored-by: Nikita Sobolev <[email protected]>
@AlexWaygood @hugovk
simpler glob
773c2ba 
Co-authored-by: Hugo van Kemenade <[email protected]>
@AlexWaygood
Address various reviews!
6c7ecb9
@AlexWaygood
AlexWaygood requested review from JelleZijlstra and hugovk May 12, 2023 13:37
hugovk
hugovk reviewed May 12, 2023
View reviewed changes
Comment thread .github/workflows/mypy.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread .github/workflows/mypy.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/clinic/requirements-dev.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood @hugovk
more from Hugo
717ee47 
Co-authored-by: Hugo van Kemenade <[email protected]>
AlexWaygood
AlexWaygood commented May 12, 2023
View reviewed changes
Comment thread Tools/clinic/requirements-dev.txt
@@ -0,0 +1,2 @@
# Requirements file for external linters and checks we run on Tools/clinic/ in CI
mypy==1.2.0

@AlexWaygood AlexWaygood May 12, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We're deliberately pinning to an older version here, in order to test that dependabot updates are working on this file, as per @hugovk's suggestion in #104421 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@AlexWaygood
Fix ini syntax
62669d1
@AlexWaygood AlexWaygood mentioned this pull request May 12, 2023
GH-86275: Implementation of hypothesis stubs for property-based tests, with zoneinfo tests #22863
Merged
hugovk
hugovk approved these changes May 12, 2023
View reviewed changes

@hugovk hugovk left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

CI changes look good 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bedevere-bot bedevere-bot removed the awaiting core review label May 12, 2023
@bedevere-bot bedevere-bot added the awaiting merge label May 12, 2023
AlexWaygood
AlexWaygood commented May 12, 2023
View reviewed changes
Comment thread Tools/clinic/cpp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
Unused import
ddd0843
@AlexWaygood

AlexWaygood commented May 12, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

(The test_zoneinfo failure is obviously unrelated.)

Edit: As is the test_threading failure.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
Merge branch 'main' into clinic-mypy-check
b7158db
JelleZijlstra
JelleZijlstra reviewed May 13, 2023
View reviewed changes
Comment thread Tools/clinic/mypy.ini Resolved

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood added 2 commits May 13, 2023 12:19
@AlexWaygood
Restore compatibility with Python 3.8
9fcd87f
@AlexWaygood
Revert "Restore compatibility with Python 3.8"
010f0dd 
This reverts commit 9fcd87f.
erlend-aasland
erlend-aasland approved these changes May 15, 2023
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood reacted with heart emoji AlexWaygood reacted with rocket emoji
All reactions
  • ❤️ 1 reaction
  • 🚀 1 reaction
@AlexWaygood

AlexWaygood commented May 15, 2023

Copy link
Copy Markdown
Member Author

LGTM. Thanks!

Very happy to help!
erlend-aasland reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
AlexWaygood enabled auto-merge (squash) May 15, 2023 08:29
@AlexWaygood
AlexWaygood merged commit 9d41f83 into python:main May 15, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label May 15, 2023
@sobolevn sobolevn mentioned this pull request May 15, 2023
Run mypy on cases_generator #104504
Closed
@AlexWaygood
AlexWaygood deleted the clinic-mypy-check branch May 15, 2023 10:22
carljm added a commit to carljm/cpython that referenced this pull request May 15, 2023
@carljm
Merge branch 'main' into supermon
343867a 
* main: (29 commits)
  pythongh-101819: Fix _io clinic input for unused base class method stubs (python#104418)
  pythongh-101819: Isolate `_io` (python#101948)
  Bump mypy from 1.2.0 to 1.3.0 in /Tools/clinic (python#104501)
  pythongh-104494: Update certain Tkinter pack/place tests for Tk 8.7 errors (python#104495)
  pythongh-104050: Run mypy on `clinic.py` in CI (python#104421)
  pythongh-104490: Consistently define phony make targets (python#104491)
  pythongh-67056: document that registering/unregistering an atexit func from within an atexit func is undefined (python#104473)
  pythongh-104487: PYTHON_FOR_REGEN must be minimum Python 3.10 (python#104488)
  pythongh-101282: move BOLT config after PGO (pythongh-104493)
  pythongh-104469 Convert _testcapi/float.c to use AC (pythongh-104470)
  pythongh-104456: Fix ref leak in _ctypes.COMError (python#104457)
  pythongh-98539: Make _SSLTransportProtocol.abort() safe to call when closed (python#104474)
  pythongh-104337: Clarify random.gammavariate doc entry  (python#104410)
  Minor improvements to typing docs (python#104465)
  pythongh-87092: avoid gcc warning on uninitialized struct field in assemble.c (python#104460)
  pythonGH-71383: IDLE - Document testing subsets of modules (python#104463)
  pythongh-104454: Fix refleak in AttributeError_reduce (python#104455)
  pythongh-75710: IDLE - add docstrings and comments to editor module (python#104446)
  pythongh-91896: Revert some very noisy DeprecationWarnings for `ByteString` (python#104424)
  Add a mention of PYTHONBREAKPOINT to breakpoint() docs (python#104430)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 15, 2023
@carljm
Merge branch 'main' into cached_property_docs
452d9f5 
* main: (204 commits)
  pythongh-101819: Fix _io clinic input for unused base class method stubs (python#104418)
  pythongh-101819: Isolate `_io` (python#101948)
  Bump mypy from 1.2.0 to 1.3.0 in /Tools/clinic (python#104501)
  pythongh-104494: Update certain Tkinter pack/place tests for Tk 8.7 errors (python#104495)
  pythongh-104050: Run mypy on `clinic.py` in CI (python#104421)
  pythongh-104490: Consistently define phony make targets (python#104491)
  pythongh-67056: document that registering/unregistering an atexit func from within an atexit func is undefined (python#104473)
  pythongh-104487: PYTHON_FOR_REGEN must be minimum Python 3.10 (python#104488)
  pythongh-101282: move BOLT config after PGO (pythongh-104493)
  pythongh-104469 Convert _testcapi/float.c to use AC (pythongh-104470)
  pythongh-104456: Fix ref leak in _ctypes.COMError (python#104457)
  pythongh-98539: Make _SSLTransportProtocol.abort() safe to call when closed (python#104474)
  pythongh-104337: Clarify random.gammavariate doc entry  (python#104410)
  Minor improvements to typing docs (python#104465)
  pythongh-87092: avoid gcc warning on uninitialized struct field in assemble.c (python#104460)
  pythonGH-71383: IDLE - Document testing subsets of modules (python#104463)
  pythongh-104454: Fix refleak in AttributeError_reduce (python#104455)
  pythongh-75710: IDLE - add docstrings and comments to editor module (python#104446)
  pythongh-91896: Revert some very noisy DeprecationWarnings for `ByteString` (python#104424)
  Add a mention of PYTHONBREAKPOINT to breakpoint() docs (python#104430)
  ...
erlend-aasland
erlend-aasland reviewed May 16, 2023
View reviewed changes
Comment thread Tools/clinic/clinic.py

# The C name to use for this variable.
name = None
name: str | None = None

@erlend-aasland erlend-aasland May 16, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

So, these str | None are now starting to cause frustrations, as I gradually add typing to other parts of clinic.py. Since there are no guards anywhere, I obviously get errors and warnings about str plus None, etc. We could change the defaults to the empty string, but that can also create a lot of havoc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@sobolevn sobolevn sobolevn left review comments
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@hugovk hugovk hugovk approved these changes
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@AlexWaygood @JelleZijlstra @hugovk @sobolevn @erlend-aasland @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.