Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1637 +/- ##
===========================================
+ Coverage 52.73% 79.04% +26.30%
===========================================
Files 37 37
Lines 4399 4390 -9
===========================================
+ Hits 2320 3470 +1150
+ Misses 2079 920 -1159 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
I think we should keep skipping the tests/files directory. The content there is generated programmatically, and excluding it helps ensure we don't accidentally modify generated artifacts.
Not sure, but after this change it might make sense to adjust pre-commit-config.yaml or pyproject.toml make the skip rule less restrictive.
Also please sync with main, there are some conflicts.
Other than that, the rest of the changes look good to me.
I think Franz was the one who told me to include the |
|
|
||
| [[tool.mypy.overrides]] | ||
| module = ["tests.*", "openml.extensions.sklearn.*"] | ||
| module = [ |
There was a problem hiding this comment.
we should track this in an issue that the following files are being skipped by pre-commit, and should be updated with time.
There was a problem hiding this comment.
I'll open an issue if you'd like me too.
There was a problem hiding this comment.
yes please that would be nice, also mention how could one reproduce these failures, probably remove this file from the list right?
There was a problem hiding this comment.
what do you mean by "this" file over here?
There was a problem hiding this comment.
Opened an issue here. It could serve as a good first issue for new contributors, epsecially with the next ESoC around.
There was a problem hiding this comment.
what do you mean by "this" file over here?
I mean to reproduce these failures for a particular file one would simply remove for-example tests.test_datasets.test_dataset from the list module in pyproject.toml, right?
fkiraly
left a comment
fkiraly
left a comment
There was a problem hiding this comment.
@PGijsbers should check whether the changes in the precommits are fine with the entire team
PGijsbers
left a comment
PGijsbers
left a comment
There was a problem hiding this comment.
LGTM; small question though
| files: ".*" | ||
| - id: check-yaml | ||
| files: ".*" | ||
| exclude: ^mkdocs\.yml$ |
There was a problem hiding this comment.
Removed it earlier because of the errors occurring here. I thought I should leave it untouched given I did not plan on making any "real" changes with this PR.
5 participants
fixes #1639.
Only major changes in this PR:
ruffversion update from v0.14.10 to v0.15.1mypyversion update from v1.13.0 to v1.19.1mypy(instead of using the suggested approach by @fkiraly to only check for files changed and then chip at it brick-by-brick to fix mypy errors, the architecture of those functions would most likely need to be changed, so if someone was adding a simple test they'd have to fix a ton of errors occuring in that file for other functions, which doesnt make sense)testsdirectory in ruff and mypy checks:ruffwherever possible and addednoqatags to other placesmypy(for the reason mentioned above in point 2)