Skip to content

Add back pre-commit job for mypy type-checking#6827

Merged
agoscinski merged 10 commits into
aiidateam:mainfrom
danielhollas:pre-commit-job
May 1, 2025
Merged

Add back pre-commit job for mypy type-checking#6827
agoscinski merged 10 commits into
aiidateam:mainfrom
danielhollas:pre-commit-job

Conversation

@danielhollas
Copy link
Copy Markdown
Collaborator

@danielhollas danielhollas commented Apr 17, 2025

Add back pre-commit CI job that runs all pre-commit hooks, including mypy, see #6831 for details.

Here are mypy failures that are currently present on main:

src/aiida/storage/sqlite_zip/utils.py:75: error: Argument 1 to "_contains" has incompatible type "str | bytes | bytearray | dict[Any, Any] | list[Any]"; expected "dict[Any, Any] | list[Any]"  [arg-type]
src/aiida/storage/sqlite_zip/utils.py:75: error: Argument 2 to "_contains" has incompatible type "str | bytes | bytearray | dict[Any, Any] | list[Any]"; expected "dict[Any, Any] | list[Any]"  [arg-type]
Found 2 errors in 1 file (checked 457 source files)

I wasn't sure how to fix them, as they seem to be false-positives so I just added type-ignores for now.

Closes #6831

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.44%. Comparing base (936185b) to head (ea3213e).
⚠️ Report is 56 commits behind head on main.

Files with missing lines Patch % Lines
src/aiida/orm/nodes/node.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6827      +/-   ##
==========================================
- Coverage   78.45%   78.44%   -0.00%     
==========================================
  Files         568      568              
  Lines       43092    43093       +1     
==========================================
  Hits        33802    33802              
- Misses       9290     9291       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread .pre-commit-config.yaml
pyproject.toml|
utils/dependency_management.py
utils/dependency_management.py|
environment.yml|
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If somebody modifies environment.yml manually, this hook should run.

Comment thread environment.yml Outdated
@danielhollas danielhollas marked this pull request as ready for review April 22, 2025 23:18
@danielhollas danielhollas self-assigned this Apr 22, 2025
danielhollas and others added 6 commits April 30, 2025 17:09
Since we do type checking in this job we need to the lowest supported
version otherwise the type checker suggests improvements that are not
possible in 3.9.
Copy link
Copy Markdown
Collaborator

@agoscinski agoscinski left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I rebased on main and fixed the new type checking error as far as possible. There is an inherent problem in the pydantic model PR #6255 that does not allow a fix of the type checks. We merged the PR as an experimental feature and I marked this as an issue in #6842. @danielhollas Could you review my changes then I merge.

This ensures that mypy does not add suggestions that are not supported
by python 3.9 while still being able to use mypy with a higher python
version.
If the change is not breaking the 3.9 CI, it should be fine. We maybe
take the advantage to upgrade it to newest version to use new features.
@agoscinski
Copy link
Copy Markdown
Collaborator

agoscinski commented Apr 30, 2025

Sorry for the additional changes but the version change did not work so I had to find a different solution (changing it back at the end^^). I finished now.

Copy link
Copy Markdown
Collaborator Author

@danielhollas danielhollas left a comment

Choose a reason for hiding this comment

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

@agoscinski thanks for the rebase and fixes. LGTM with just one comment. Hopefully we can get rid of the type-ignores in later PRs, but for now it's I think important to get this in to prevent further type-checking problems and to unblock other type-checking PRs.

Comment thread .pre-commit-config.yaml Outdated
Co-authored-by: Daniel Hollas <danekhollas@gmail.com>
@agoscinski agoscinski merged commit 6eb17a7 into aiidateam:main May 1, 2025
27 checks passed
@danielhollas danielhollas deleted the pre-commit-job branch May 1, 2025 08:43
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.

mypy type check does not run in CI

2 participants