ci: compare three type checker#25502
ci: compare three type checker#25502asukaminato0721 wants to merge 56 commits intolanggenius:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the GitHub Actions workflow to use zmypy instead of mypy for Python type checking. The change involves installing the zuban package and replacing the mypy command with zmypy while maintaining all the same command-line arguments and type checking configuration.
- Replaces the mypy type checking command with zmypy in the CI pipeline
- Adds installation of the zuban package as a dependency for zmypy
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bb106a4 to
ab51f90
Compare
ab51f90 to
acdd901
Compare
|
well |
88646a4 to
f9153d5
Compare
f9153d5 to
41a2530
Compare
|
Added RUST_BACKTRACE environment variable for Mypy checks.
|
This project be a profiler |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| run: dev/basedpyright-check | ||
|
|
||
|
|
There was a problem hiding this comment.
The PR removes the call to make type-check which includes mypy (as shown in Makefile:71-74), but mypy is not being run in the new workflow steps. This means mypy type checking is no longer being executed in CI. If the goal is to compare type checkers while still running the existing checks, consider keeping mypy or explicitly documenting why it was removed. The Makefile's type-check target runs: basedpyright, mypy, and ty.
| - name: Run mypy Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| run: uv run --directory api mypy . |
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | ||
|
|
||
| - name: Run zmypy Type Checks |
There was a problem hiding this comment.
The step name says "Run zmypy Type Checks" but the command uses the package "zuban". These should be consistent. Based on the issue reference (https://zubanls.com/), the correct package name is "zuban", so the step name should be updated to "Run zuban Type Checks" for clarity and consistency.
| - name: Run zmypy Type Checks | |
| - name: Run zuban Type Checks |
| run: uv --directory api add ty && uv --directory api run ty check || true | ||
|
|
||
| - name: Run pyrefly Type Checks | ||
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| env: | ||
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | ||
|
|
||
| - name: Run zmypy Type Checks | ||
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| env: | ||
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add zuban && uv --directory api run zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . |
There was a problem hiding this comment.
Using uv --directory api add will modify the project's dependencies by adding these packages to pyproject.toml and uv.lock. For testing purposes, consider using uv --directory api run --with ty ty check instead, which installs the package temporarily without modifying the project files. This is more appropriate for experimental CI checks that shouldn't affect the project's dependency declarations.
| run: uv --directory api add ty && uv --directory api run ty check || true | |
| - name: Run pyrefly Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | |
| - name: Run zmypy Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api add zuban && uv --directory api run zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . | |
| run: uv --directory api run --with ty ty check || true | |
| - name: Run pyrefly Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api run --with pyrefly pyrefly check || true | |
| - name: Run zmypy Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api run --with zuban zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . |
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| env: | ||
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true |
There was a problem hiding this comment.
Using uv --directory api add will modify the project's dependencies by adding these packages to pyproject.toml and uv.lock. For testing purposes, consider using uv --directory api run --with pyrefly pyrefly check instead, which installs the package temporarily without modifying the project files. This is more appropriate for experimental CI checks that shouldn't affect the project's dependency declarations.
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | |
| run: uv --directory api run --with pyrefly pyrefly check || true |
| run: uv --directory api add ty && uv --directory api run ty check || true | ||
|
|
||
| - name: Run pyrefly Type Checks | ||
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| env: | ||
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | ||
|
|
||
| - name: Run zmypy Type Checks | ||
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| env: | ||
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add zuban && uv --directory api run zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . |
There was a problem hiding this comment.
Using uv --directory api add will modify the project's dependencies by adding these packages to pyproject.toml and uv.lock. For testing purposes, consider using uv --directory api run --with zuban zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . instead, which installs the package temporarily without modifying the project files. This is more appropriate for experimental CI checks that shouldn't affect the project's dependency declarations.
| run: uv --directory api add ty && uv --directory api run ty check || true | |
| - name: Run pyrefly Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | |
| - name: Run zmypy Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api add zuban && uv --directory api run zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . | |
| run: uv --directory api run --with ty ty check || true | |
| - name: Run pyrefly Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api run --with pyrefly pyrefly check || true | |
| - name: Run zmypy Type Checks | |
| if: steps.changed-files.outputs.any_changed == 'true' | |
| env: | |
| RUST_BACKTRACE: 1 | |
| run: uv --directory api run --with zuban zuban check --exclude '.venv/' --exclude 'tests/' --exclude 'migrations/' --check-untyped-defs --disable-error-code=import-untyped . || true |
| env: | ||
| RUST_BACKTRACE: 1 | ||
| run: uv --directory api add pyrefly && uv --directory api run pyrefly check || true | ||
|
|
There was a problem hiding this comment.
This line contains trailing whitespace which should be removed for consistency with the project's style standards.
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| run: dev/basedpyright-check | ||
|
|
||
|
|
There was a problem hiding this comment.
This line contains trailing whitespace which should be removed for consistency with the project's style standards.
Important
Fixes #<issue number>.fix #25187
Summary
just to test the result.
Screenshots
Checklist
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods