Conversation
adbc918 to
294ec62
Compare
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 21 files ±0 21 suites ±0 5h 34m 2s ⏱️ + 4m 13s Results for commit 6d8bd67. ± Comparison against base commit ac02b7d. ♻️ This comment has been updated with latest results. |
1d8b834 to
066e34f
Compare
| "error:::dask[.*]", | ||
| "error:::pandas[.*]", | ||
| "error:::numpy[.*]", | ||
| "error:::distributed[.*]", |
There was a problem hiding this comment.
To my understanding, the intention here was to avoid having to explicitly suppress warnings caused by third-party libraries.
However, I've observed quite a few cases of uncaught Pandas4Warning that were definitely of dask's compentence, e.g. #12272.
There was a problem hiding this comment.
My understanding is that this converts any warning from dask, pandas, numpy, distributed to an error. So it's the opposite of suppressing. But I think having all warnings error like you have in the PR is also fine.
066e34f to
4fd8050
Compare
| - scipy=1.7.2 | ||
| - scikit-learn=1.0.2 | ||
| - scipy=1.10.0 | ||
| - setuptools=60 |
There was a problem hiding this comment.
There are three different deprecation warnings re. distutils on
- setuptools <60,
- setuptools >=60,<81,
- setuptools >=81
59e2d83 to
636e77d
Compare
684135b to
788578c
Compare
788578c to
684135b
Compare
684135b to
e8a3cb4
Compare
|
@jsignell Would you have time to review this? |
| "error:::dask[.*]", | ||
| "error:::pandas[.*]", | ||
| "error:::numpy[.*]", | ||
| "error:::distributed[.*]", |
There was a problem hiding this comment.
My understanding is that this converts any warning from dask, pandas, numpy, distributed to an error. So it's the opposite of suppressing. But I think having all warnings error like you have in the PR is also fine.
| 'ignore:The previous implementation of stack is deprecated and will be removed in a future version of pandas\.:FutureWarning', | ||
| "ignore:distutils Version classes are deprecated. Use packaging.version instead.:DeprecationWarning", | ||
| # FIXME spurious shutdown of distributed workers and/or sqlite3 | ||
| "module:Exception ignored:pytest.PytestUnraisableExceptionWarning::", |
There was a problem hiding this comment.
It seems weird to ignore pytest warnings. Is the plan to fix these when there is more maintenance time available?
There was a problem hiding this comment.
Yes, these need to be looked at eventually.
There was a problem hiding this comment.
Could you open an issue to track this?
That was my naive understanding as well, but it resulted in a big bunch of Pandas4Warning to remain suppressed, so there must be something wrong with that logic. |
Revisit
filterwarningsinpyproject.tomlto be stricter, causing a hard error on all unexpected warnings.This PR was prompted by noticing that
Pandas4Warning's do not always cause a hard error in the test suite, whereas readingpyproject.tomlwould suggest the opposite. Also in the past (#12052) there were several deprecations that were uncaught until they finally broke.This PR builds on top of
and should be merged after them. Please only look at the last commit.