Fix mypy errors in core.py - chunk_reduce #154
Conversation
Illviljan
changed the title
| dict | ||
| """ | ||
|
|
||
| if not (isinstance(func, str) or callable(func)): |
There was a problem hiding this comment.
I llike this but we shoudl do this at the groupby_reduce level. Otherwise for dask stuff it will only raise at compute time. Unless you're checking that we've created Aggregation objects correctly.
There was a problem hiding this comment.
This is to make sure funcs is always a sequence. chunk_reduce is used on too many places though, it would require too many unrelated typing changes. I think it's for another PR once the mypy CI is active.
| dtypes = dtype | ||
| else: | ||
| dtypes = (dtype,) * nfuncs | ||
| assert len(dtypes) >= nfuncs |
There was a problem hiding this comment.
| assert len(dtypes) >= nfuncs | |
| assert len(dtypes) == nfuncs |
IMO we should be more strict here. This looseness has resulted in bugs/confusions before
There was a problem hiding this comment.
Ok, I tried it out and some parts of the code actually relies on this behaviour. >= was in the original code so I'm going to leave it the same in this PR.
Splitting up #150 as I had trouble finding typos.