Repository navigation
Pin mypy==1.18.2 - #2087
Pin mypy==1.18.2#2087
mypy==1.18.2#2087Conversation
There was a problem hiding this comment.
Pull request overview
This PR pins mypy to version 1.18.2 and addresses type checking errors to enable strict type checking in CI. The changes remove continue-on-error: true from the mypy CI step and restrict mypy execution to Python 3.9+, ensuring that type errors will now cause CI failures.
- Pins mypy version and restricts it to Python 3.9+ environments
- Adds specific type ignore comments with error codes for legitimate type checking edge cases
- Fixes type annotations and removes unnecessary casts
- Prevents pytest from treating test utility classes as test classes
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test-requirements.txt | Pins mypy==1.18.2 for Python 3.9+ to prevent new type errors from future mypy releases |
| pyproject.toml | Removes python_version constraint allowing mypy to use the running Python version |
| .github/workflows/pythonpackage.yml | Restricts mypy checks to Python 3.9+ and removes continue-on-error to fail CI on type errors |
| test/test_submodule.py | Adds __test__ = False to prevent pytest treating TestRootProgress as a test class and adds noqa for assertRaises |
| test/test_remote.py | Adds return type annotation and __test__ = False to TestRemoteProgress |
| test/lib/helper.py | Removes unnecessary B001 noqa code from bare except statements |
| test/deprecation/test_types.py | Removes type ignore comment now that Lit_commit_ish is properly defined |
| git/types.py | Guards Lit_commit_ish type alias with TYPE_CHECKING for proper deprecation handling |
| git/util.py | Adds type ignores for IterableList, fixes isinstance check logic, and improves error messages |
| git/repo/fun.py | Removes unnecessary cast from ref.commit |
| git/repo/base.py | Simplifies config reader comprehension and adds type ignores for progress argument |
| git/refs/tag.py | Moves type ignore comments to property decorator line for better clarity |
| git/refs/symbolic.py | Replaces explicit type annotation with cast for instance variable |
| git/refs/log.py | Adds type ignore for RefLogEntry tuple construction |
| git/refs/head.py | Removes duplicate commit type annotation and adds type ignore for delete override |
| git/objects/submodule/base.py | Adds RemoteReference cast for tracking_branch() result and imports RemoteReference |
| git/objects/commit.py | Adds str() cast for message in regex to handle bytes type |
| git/index/typ.py | Adds type ignore comments for IndexEntry tuple construction |
| git/diff.py | Adds type annotation for change_type class variable with noqa comment |
| git/config.py | Updates format string to f-string, removes unnecessary cast and comment, and uses cp._SectionName type |
Comments suppressed due to low confidence (1)
git/util.py:1210
- This method raises ValueError - should raise a LookupError (KeyError or IndexError) instead.
This method raises AttributeError - should raise a LookupError (KeyError or IndexError) instead.
def __getitem__(self, index: Union[SupportsIndex, int, slice, str]) -> T_IterableObj: # type: ignore[override]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot for making this happen!
Apologies for unleashing AI as well, but I think it can be helpful given that I don't really know python anymore, and am quite clueless 😅.
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot, let's do it then!
The `Check types with mypy` step in the `test` job of `pythonpackage.yml` ran only `if: matrix.python-version != '3.8'`. That condition was added in gitpython-developers#2087, alongside a `python_version >= "3.9"` marker on mypy in `test-requirements.txt`. Since mypy was not installed on older Pythons, the step had to be skipped there. (The same pull request also pinned mypy to 1.18.2, which itself requires Python 3.9 or later.) The condition originally excluded 3.7 as well, and that part was removed along with 3.7 support. Since Python 3.8 through 3.10 were moved out of the `test` job's matrix into the `install/import` job, which does not run mypy, the `test` job runs only on 3.11 through 3.15t. No row of its matrix, and no `include` entry, uses 3.8, so the condition is always true. It suggested that the job still covers 3.8 and did nothing else. Remove it. The step runs in exactly the same jobs as before. The `python_version >= "3.9"` marker on the mypy requirement is left as it is. It is packaging metadata for the `test` extra, not CI configuration. `actionlint` reports no problems, and zizmor 1.31.1 at the `pedantic` level reports no new findings. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The tox `mypy` environment has `ignore_outcome = true`, so a failure there is reported but does not fail the run. 4bea7cf (gitpython-developers#1667) added that to match CI, where the mypy step had `continue-on-error: true` because GitPython was not expected to pass mypy at the time. But 50762f1 (gitpython-developers#2087) removed `continue-on-error` from that step, so a mypy failure has failed CI since then, and mypy passes there on every Python version the `test` job covers. The tox environment was never updated to match. Now that tox also runs in the weekly `tox` workflow, this matters more: as things were, that workflow could not fail because of the `mypy` environment, even if the environment could not be set up at all. Remove `ignore_outcome`, so `mypy` failures fail tox as they fail CI. The `mypy` environment passed, with `ignore_outcome` false, on each of Python 3.9.6, 3.10.22, and 3.13.17, selected with `-x tool.tox.env.mypy.base_python=py39` and so on ("Success: no issues found in 40 source files"). 3.9 and 3.10 are in its `base_python` list, though CI only type-checks on 3.11 and later. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Pin mypy version and fix errors
Now, the CI will fail when the types are incorrect