-
-
Notifications
You must be signed in to change notification settings - Fork 210
[MNT] extend CI to newer python versions, deprecate python versions 3.8, 3.9 after EOL, marking further failing tests as xfail
#1579
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
Conversation
xfail
|
FYI @JATAYU000, I have merged #1571 into this for simplicity. This also allows us to track down any further failing tests. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #1579 +/- ##
===========================================
- Coverage 85.24% 52.70% -32.54%
===========================================
Files 38 36 -2
Lines 5008 4320 -688
===========================================
- Hits 4269 2277 -1992
- Misses 739 2043 +1304 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fkiraly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a few failing tests, and also fixture construction errors - could you look at those too, @JATAYU000?
Yes will look into it |
|
ok, I will also check whether that resolves the failures here. |
|
this fixed the issue, so you should remove the Not in this PR, but in #1584, and I will merge the changes to here as well for more extensive testing. |
#### Metadata * Reference Issue: Refer failures in #1579 * New Tests Added: No * Documentation Updated: No * Change Log Entry: Checks if the directory was created newly else doesn't remove. ### Details * What does this PR implement/fix? Explain your changes. `get_task` checks if the `tid_cache_dir` was already existing before removing it on `Exception` * Why is this change necessary? What is the problem it solves? `OpenMLServerException` causes `get_task` to remove the entire directory even if the directory was already existing and is used by other tests * How can I reproduce the issue this PR is solving and its solution? observe `exists assertion` errors for files under `tests/files/org/openml/test/task/1/` after running `pytest` or look at failures in #1579
|
A PR by @PGijsbers got merged which seems to fix these. Unfortunately, this was unexpected since I thought he was on holiday until Jan - but it seems to be a fix, so we should adapt this PR to the new repository state. I would suggest to revert the tests that are now fixed, could you quickly do that? |
Sure, Thank you |
|
@JATAYU000, you still need to deal with the |
Yes those tests marks are removed, rest are passing till now the check is still in progress |
@fkiraly Can you point me which tests under |
fkiraly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we have the failing tests under control now.
The CI runs only on python versions 3.8 and 3.9 both of which have already reached end of life.
This PR updates the python versions, deprecating any logic that runs tests on python versions 3.8 and 3.9, or
scikit-learnversions of that age.Metadata
Reference Issue: #1544
Depends on #1584 fofr a fix, which should be merged first.
Details
mark.xfailwithreason="failures_issue_1544"for all the remaining failed tests (after [BUG] Temporarily fix issue #1544 by marking the failed tests as expected fail. #1572) in issue: [MNT] Test failures in CI and local #1544