feat: Replace MyPy with ty type checker (do not merge)#728
feat: Replace MyPy with ty type checker (do not merge)#728Aaron ("AJ") Steers (aaronsteers) wants to merge 5 commits into
Conversation
- Replace mypy dependency with ty ^0.0.1a16 in pyproject.toml - Remove mypy configuration section and add ty.src configuration - Update CI workflow from mypy-check to ty-check - Create generic type-check Poe task using ty check command - Update existing check task to use new type-check task - Exclude tests, docs, and fixture directories from ty checking This change enables testing of the new Rust-based ty type checker as an alternative to mypy for performance and usability evaluation. Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Original prompt from AJ Steers: |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1738195530-test-ty-type-checker' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1738195530-test-ty-type-checker'Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
Community SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe Python project's type checking setup was migrated from MyPy to Ty. This involved updating the GitHub Actions workflow, replacing MyPy commands and configuration with Ty, and modifying the project's dependency and task definitions in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Would you like to also update any related documentation or developer onboarding instructions to reflect the switch from MyPy to Ty, wdyt? Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
pyproject.toml (1)
56-70:pytest-mypystill listed – will CI now error out?With
mypygone, thepytest-mypyplugin (Line 69) will attempt to invoke a missing binary and fail the test phase.
Would it make sense to drop this dependency altogether or replace it with an equivalentpytest-ty(if/when one exists), so the pipeline stays green? wdyt?.github/workflows/python_lint.yml (1)
59-81: Harden the newty-checkjob with explicitpermissionsCodeQL flags that the workflow doesn’t restrict the default
GITHUB_TOKENperms.
Shall we add the minimal read-only block for defence-in-depth? wdyt?ty-check: name: Ty Check permissions: contents: read # minimal required
🧹 Nitpick comments (2)
pyproject.toml (2)
118-124: Excludingtestsfrom type-checking – deliberate or accidental?The new
[tool.ty.src]block omits the entireteststree. This mirrors many projects’ mypy setups, but it also hides type regressions inside our test code.
Should we narrow the ignore list to integration fixtures only and let unit tests stay in scope, or is the full exclusion desired for now? wdyt?
145-147: Poe task chain – consider surfacing Ty failures earlier?
checkrunsruff, thenty, then a pytest collection. Given thattycurrently emits 100+ diagnostics, would flipping the order to runpoe type-checkfirst shorten feedback for contributors?
Example:-check = { shell = "ruff check . && poe type-check && pytest --collect-only -qq" } +check = { shell = "poe type-check && ruff check . && pytest --collect-only -qq" }Keeps the gate, but fails faster. Worth adopting? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/python_lint.yml(2 hunks)pyproject.toml(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: yohannj
PR: airbytehq/PyAirbyte#716
File: airbyte/logs.py:384-402
Timestamp: 2025-07-11T19:53:44.427Z
Learning: In the PyAirbyte project, when reviewing PRs, maintain clear separation of concerns. Don't suggest changes that are outside the scope of the PR's main objective, even if they would improve consistency or fix other issues. This helps with reviewing changes and potential reverts.
pyproject.toml (2)
Learnt from: aaronsteers
PR: #347
File: tests/integration_tests/fixtures/registry.json:48-48
Timestamp: 2024-10-08T15:34:31.026Z
Learning: Test fixtures in the PyAirbyte project do not need to align with real Docker repositories.
Learnt from: aaronsteers
PR: #347
File: tests/integration_tests/fixtures/registry.json:48-48
Timestamp: 2024-08-31T01:20:08.405Z
Learning: Test fixtures in the PyAirbyte project do not need to align with real Docker repositories.
🪛 GitHub Check: CodeQL
.github/workflows/python_lint.yml
[warning] 60-80: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (No Creds)
🔇 Additional comments (1)
pyproject.toml (1)
56-64: Pre-releasetypin looks good, but should it be an exact version?
tyis still in alpha (^0.0.1a16). Minor alpha bumps can introduce breaking changes.
Do we want to freeze to==0.0.1a16for deterministic CI while we evaluate, or is the caret-range intentional to track new alphas? wdyt?
…ion issues Co-Authored-By: AJ Steers <aj@airbyte.io>
…in permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
2 participants
Replace MyPy with ty type checker (do not merge)
Summary
This PR replaces MyPy with
ty(the new Rust-based Python type checker from astral-sh) across both PyAirbyte and airbyte-python-cdk repositories for performance and usability testing. This is an experimental change to evaluate ty as an alternative to MyPy.Key changes:
mypydependency withty ^0.0.1a16in pyproject.toml[tool.ty.src]configurationmypy-checktoty-checktype-checkpoethepoet tasks usingty checkcommandsType checking results:
Note: ty appears significantly stricter than MyPy, finding many more type issues.
Review & Testing Checklist for Human
poetry run poe type-checkandpoetry run poe checkin both repositories to ensure tasks work correctlypoetry installin both repos to verify the ty dependency installs without conflicts[tool.ty.src]configuration sections are syntactically correct for ty (alpha tool, documentation may be incomplete)Recommended test plan:
poetry installpoetry run ty check .(PyAirbyte) andpoetry run ty check airbyte_cdk(airbyte-python-cdk)poetry run poe type-checkand main check tasksDiagram
%%{ init : { "theme" : "default" }}%% graph TD subgraph PyAirbyte PA_pyproject["pyproject.toml<br/>(PyAirbyte)"]:::major-edit PA_workflow[".github/workflows/<br/>python_lint.yml<br/>(PyAirbyte)"]:::major-edit PA_lock["poetry.lock<br/>(PyAirbyte)"]:::minor-edit end subgraph airbyte-python-cdk CDK_pyproject["pyproject.toml<br/>(airbyte-python-cdk)"]:::major-edit CDK_workflow[".github/workflows/<br/>python_lint.yml<br/>(airbyte-python-cdk)"]:::major-edit CDK_mypy["mypy.ini<br/>(airbyte-python-cdk)"]:::major-edit end subgraph Changes C1["Replace mypy → ty dependency"] C2["Update CI: mypy-check → ty-check"] C3["Add [tool.ty.src] config"] C4["Update poethepoet tasks"] end PA_pyproject --> C1 PA_pyproject --> C3 PA_pyproject --> C4 CDK_pyproject --> C1 CDK_pyproject --> C3 CDK_pyproject --> C4 PA_workflow --> C2 CDK_workflow --> C2 CDK_mypy --> C3 subgraph Legend L1[Major Edit]:::major-edit L2[Minor Edit]:::minor-edit L3[Context/No Edit]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
This is the first of two experimental PRs testing Rust-based Python type checkers. The second PR will test pyrefly (Meta/Facebook's type checker) for comparison.
Summary by CodeRabbit