Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mypy: ignore[assignment] is no longer needed in src/aiida/orm/utils/serialize.py::51 #6566

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

khsrali
Copy link
Contributor

@khsrali khsrali commented Sep 20, 2024

mypy is somehow magically updated 🤔

@khsrali khsrali requested a review from agoscinski September 20, 2024 11:42
@khsrali
Copy link
Contributor Author

khsrali commented Sep 20, 2024

the issue originally appeared and discovered on #6562 and #6565

@khsrali khsrali changed the title Ignore assignment is no longer needed mypy: Ignore assignment is no longer needed Sep 20, 2024
@khsrali khsrali changed the title mypy: Ignore assignment is no longer needed mypy: ignore[assignment] is no longer needed in src/aiida/orm/utils/serialize.py::51 Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.83%. Comparing base (ef60b66) to head (3e86783).
Report is 113 commits behind head on main.

Files with missing lines Patch % Lines
src/aiida/orm/utils/serialize.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6566      +/-   ##
==========================================
+ Coverage   77.51%   77.83%   +0.33%     
==========================================
  Files         560      566       +6     
  Lines       41444    41983     +539     
==========================================
+ Hits        32120    32675     +555     
+ Misses       9324     9308      -16     

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

Copy link
Contributor

@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.

Yes codecov fails because we changed one line that is not tested which results in a 0% patch coverage. I would say for the sake of simplicity and getting the other PRs merged we just bypass.

@khsrali
Copy link
Contributor Author

khsrali commented Sep 20, 2024

Really, adding more test for that line is out of scope of this PR.
I only changed the comment, I want Codecov to be smarter 🥇

@khsrali khsrali merged commit 655da5a into aiidateam:main Sep 20, 2024
10 of 11 checks passed
@khsrali khsrali deleted the mypy_annoys_me branch September 20, 2024 13:49
GeigerJ2 pushed a commit to GeigerJ2/aiida-core that referenced this pull request Sep 25, 2024
…ects (aiidateam#6566)

Apparently, somehow `mypy` is updated and it is complaining about an `ignore[assignment]` comment that previously was imposed by `mypy` itself. (in src/aiida/orm/utils/serialize.py::L51)
This commit removed `ignore[assignment]`, and `ci-style / pre-commit (pull_request)`  is passing now.
agoscinski pushed a commit to agoscinski/aiida-core that referenced this pull request Nov 5, 2024
…ects (aiidateam#6566)

Apparently, somehow `mypy` is updated and it is complaining about an `ignore[assignment]` comment that previously was imposed by `mypy` itself. (in src/aiida/orm/utils/serialize.py::L51)
This commit removed `ignore[assignment]`, and `ci-style / pre-commit (pull_request)`  is passing now.

Cherry-pick: 655da5a
agoscinski pushed a commit to agoscinski/aiida-core that referenced this pull request Nov 5, 2024
…ects (aiidateam#6566)

Apparently, somehow `mypy` is updated and it is complaining about an `ignore[assignment]` comment that previously was imposed by `mypy` itself. (in src/aiida/orm/utils/serialize.py::L51)
This commit removed `ignore[assignment]`, and `ci-style / pre-commit (pull_request)`  is passing now.

Cherry-pick: 655da5a
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.

2 participants