pre-commit: pyupgrade #1352
pre-commit: pyupgrade #1352
Conversation
Stop using deprecated type annotations and imports.
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1352 +/- ##
===========================================
+ Coverage 84.25% 84.32% +0.06%
===========================================
Files 157 157
Lines 12871 12911 +40
===========================================
+ Hits 10845 10887 +42
+ Misses 2026 2024 -2 ☔ View full report in Codecov by Sentry. |
dweindl
requested review from
stephanmg,
m-philipps,
dilpath,
PaulJonasJost,
vwiela,
FFroehlich and
Doresic
as code owners
Yes, but only after merging. |
There was a problem hiding this comment.
👍 please add 64efde1 to https://github.com/ICB-DCM/pyPESTO/blob/develop/.git-blame-ignore-revs and use ruff (UP, https://docs.astral.sh/ruff/rules/#pyupgrade-up)
.pre-commit-config.yaml
Outdated
| rev: v3.15.0 | ||
| hooks: | ||
| - id: pyupgrade | ||
| args: ["--py39-plus"] |
There was a problem hiding this comment.
Is this not automatically picked up from python-requires in setup.cfg?
There was a problem hiding this comment.
At least I didn't find that documented.
There was a problem hiding this comment.
mmh, yeah pyupgrade doesn't seem to do it out of the box. I don't think ruff reads options from setup.cfg, only pyproject.toml, but we could anyways move all of the information to a single file, right?
There was a problem hiding this comment.
but we could anyways move all of the information to a single file, right?
Yes, it would be good switch to fully switch to pyproject.toml, but that's something for another day.
After merge. Hash will change.
👍 Didn't notice it was available through ruff. |
| @@ -1,7 +1,8 @@ | |||
| import logging | |||
| import warnings | |||
| from collections.abc import Sequence | |||
There was a problem hiding this comment.
Any particular reason to use this one? Seems like just an extra import to me.
There was a problem hiding this comment.
Yes. typing.Sequence is deprecated since python 3.9.
| petab_problem: "petab.Problem", | ||
| petab_problem: petab.Problem, | ||
| model: AmiciModel, | ||
| edatas: list["amici.ExpData"], | ||
| edatas: list[amici.ExpData], |
There was a problem hiding this comment.
I thought that adding " " on petab and amici objects was good for tests not to break if the module is imported, but petab and amici are not installed. I think I tried having it like this, but then tests were failing. Why is it ok to change now?
There was a problem hiding this comment.
tl;dr: from __future__ import annotations
details: https://peps.python.org/pep-0563/
Stop using deprecated type annotations and imports.