CI: Unpin pytest #35272
CI: Unpin pytest #35272
Conversation
simonjayhawkins
added
CI
|
Hello @simonjayhawkins! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-07-29 14:30:49 UTC |
|
Nice work. Are you planning to drop some of the type ignores on the next RC? I see some of the upstream issues you've opened have been closed |
we could release the pin on @bluetech has added tests for the fixed issues so shouldn't be an issue, but could also change the |
There are basically three issues. two are fixed. I still need to look into the other. pytest-dev/pytest#7495 |
| @@ -1332,7 +1333,8 @@ def test_series_groupby_on_2_categoricals_unobserved_zeroes_or_nans( | |||
| # Tests whether the unobserved categories in the result contain 0 or NaN | |||
|
|
|||
| if reduction_func == "ngroup": | |||
| pytest.skip("ngroup is not truly a reduction") | |||
| # https://github.com/pytest-dev/pytest/issues/7495 | |||
| pytest.skip("ngroup is not truly a reduction") # type: ignore | |||
There was a problem hiding this comment.
tbh here, this is only causing errors since the function is being checked since we have type annotations in the function signature.
I think our policy is that we don't type check the tests themselves.
maybe I should remove the type annotations here instead. @WillAyd
There was a problem hiding this comment.
@WillAyd because have warn_unused_ignores = True we need to have pytest 6.0.0 as the minimum version in the dev environment if keep these ignores. I think that removing the type annotations on these tests is probably best.
There was a problem hiding this comment.
removed type annotations from pandas/tests/groupby/test_categorical.py. will remove pytest pin from environment.yml after ci run.
|
@TomAugspurger I've labelled this 1.2 for now. Is there a benefit to unpinning on 1.1.x? |
|
test failures unrelated. The ci environments are resolving to pytest 6.0.0 in Linux py37_locale and Windows py36_np15 on azure, CI / Checks on GitHub actions so I think this is good to go. @TomAugspurger |
|
Actually, I'm second-guessing not backporting this. I think we'll want a consistent pytest version for both branches being actively developed. What do you think? |
sure. happy to backport. should be a benefit to users of 1.1.1 onwards (will be interesting to see if we get any issues on 1.1.0 regarding the pin, now that pytest 6.0.0 is released.) |
|
Great, thanks. |
|
@meeseeksdev backport to 1.1.x |
Co-authored-by: Simon Hawkins <[email protected]>
have made a start working through the failures. we should be able to get mypy to green here and then when pytest 6.0.0 is released the upstream fixes should cause ci to go red since we have
warn_unused_ignores = Truein setup.cfg.