CI: unpin numpy for CI / Checks github action
State: closed

xref #39513

Labels:
name: CI
description: Continuous Integration
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
Cool thanks for taking a look. Are you expecting this new environment to be part of CI or to just leave here for visibility as we clean up the existing failures?
> Cool thanks for taking a look. Are you expecting this new environment to be part of CI or to just leave here for visibility as we clean up the existing failures?

open to suggestions.

vendoring stub or adding additional steps to current check (update environment and run mypy again) are other options.
@simonjayhawkins is getting this passing viable?  a bunch of the complaints are about e.g. `np.random.randn`, not clear how to resolve
Hello @simonjayhawkins! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



























































































There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-10-03 15:46:30 UTC
> a bunch of the complaints are about e.g. `np.random.randn`, not clear how to resolve

probably need to submit some fixes upstream

> is getting this passing viable?

imo not with the current review process, even though I normally put the errors in the OP for typing fixes

I'm now going to add the errors directly to code here, so it that it may be easier to identify the problems (and the low hanging fruit)

I may open another PR that sits on top of this with the fixes so that it is easier to see the reason for the code changes from the diff

(if this process is more successful, could maybe do the same for check-untyped-defs, many fixes were sorted 12 months ago, trying to get them into the codebase is a nightmare)
Looks like a lot of unused "type:ignore" complaints, but otherwise mypy is happy?
> > a bunch of the complaints are about e.g. `np.random.randn`, not clear how to resolve
> 
> probably need to submit some fixes upstream

has now been fixed upstream
> Looks like a lot of unused "type:ignore" complaints, but otherwise mypy is happy?

the ignores are with numpy stable. Found 402 errors in 81 files. That's the number of ignores to get mypy to pass with numpy nightly. (added as separate ci job)

we should have a discussion at some point how we plan to 'switch' to using numpy 1.20 for type checking once released.

> > Looks like a lot of unused "type:ignore" complaints, but otherwise mypy is happy?
> 
> the ignores are with numpy stable. Found 402 errors in 81 files. That's the number of ignores to get mypy to pass with numpy nightly. (added as separate ci job)
> 
> we should have a discussion at some point how we plan to 'switch' to using numpy 1.20 for type checking once released.

xref #37740 reduces to Found 312 errors in 75 files if we can get buy-in for change AnyArrayLike and ArrayLike to Union instead of TypeVars.  https://github.com/pandas-dev/pandas/pull/37224#discussion_r512859195
>  if we can get buy-in for change AnyArrayLike and ArrayLike to Union instead of TypeVars

I'm on board.
#37771 fixed one of the mypy errors here. There may be more errors hidden from mypy on master because numpy types currently resolve to `Any` that are not false positives once we can remove much of the noise here.
I've added the 1.2.2 milestone. see https://github.com/pandas-dev/pandas/issues/39513#issuecomment-771006563

This PR is moreless just comments, so should be no harm backporting

an alternative maybe to modify the ci script on 1.2.x to use a different env from environment.yml  or to not follow imports when mypy checking
This gets mypy to green. 

benchmarks failing, see  https://github.com/pandas-dev/pandas/pull/38379#discussion_r567753143

need to deal with that (as a pre-cursor) before unpining numpy
if you would merge master
moving to 1.2.3
urgh. IsInLongSeriesValuesDominate is also failing. didn't notice that before.
green. 

have added the benchmark skip here https://github.com/pandas-dev/pandas/pull/36092/commits/4068a38c858bc4be81efb4a66e6ec7218594c847. this could be a pre-cursor. ( addresses @jorisvandenbossche follow-up comment https://github.com/pandas-dev/pandas/pull/39795#discussion_r576792892)


@simonjayhawkins we want to backport this?
I don't think so, but we should remove the pins from environment.yml on 1.2.x at the same time. will need to change the ci workflow file to use the old numpy or not follow imports for typing. 

This can be done after or before this.
so the issue this is addressing should remain milestoned 1.2.x and this issue does not close it.
> I don't think so, but we should remove the pins from environment.yml on 1.2.x at the same time. will need to change the ci workflow file to use the old numpy or not follow imports for typing.
> 
> This can be done after or before this.

#39998 and #39994
this lgtm @simonjayhawkins rebase once more just to be sure and let's merge!

ok. will merge shortly if green (hopefully will be, one 1 commit to master since last rebase)

There will likely be some ci errors with merging subsequent PRs that are not rebased to this one. I _think_ that we could take the hit and follow-up with ci fixes rather than asking too many PR authors to rebase.
thanks @simonjayhawkins very nice!
> ok. will merge shortly if green (hopefully will be, one 1 commit to master since last rebase)
> 
> There will likely be some ci errors with merging subsequent PRs that are not rebased to this one. I _think_ that we could take the hit and follow-up with ci fixes rather than asking too many PR authors to rebase.

yep we'll see and that's fine too