[ENH] More type-stubs in the mypy pre-commit environment?

[wence-]

Following on from #11640 (specifically #11640 (comment)) a question arises as to how complete we should make the mypy pre-commit environment in terms of supported typestubs.

Status quo:

The version of mypy installed via pre-commit only depends on the stdlib typeshed stubs (and after #11640 types-cachetools). We set ignore_missing_imports = True which means that mypy doesn't complain if it sees an import for something (say numpy) that it can't find. Consequently, any non-importable modules are typed as Any (as are all objects, methods, functions, etc... from that module); this is a type that always satisfies any type constraint.

In the development environment, all cudf modules are importable, and so a type-checking run using that environment will deduce narrower types for many function calls in cudf. Many of these currently do not type-check, being of the following form: https://mypy-play.net/?mypy=latest&python=3.10&gist=8d3ba6046bb8ca39c6d6b71b442b432c

from typing import Any

def foo(x: Any) -> Any:
    if isinstance(x, bool):
        y = x
    elif isinstance(x, int):
        y = (True, x)
    else:
        y = x

    return y

mypy complains about the assignment y = (True, x) "error: Incompatible types in assignment (expression has type "Tuple[bool, int]", variable has type "bool")" because it deduces the type of y from the first assignment as bool.

There are various places in the codebase where we do this kind of untagged union dispatch, this could be fixed by explicitly typing all the variables as Union[a, b, c, ...] but I am not sure that in the end it would be worth it. If we install numpy and the pandas-stubs package (which provides type stubs for pandas) then we get about 120 errors of this nature.

I think that fixing these things is rather difficult, the right approach is to use tagged unions, but there's no support for that in python and any workaround would (I think) make the code unnecessarily non-idiomatic.

If we think it's worthwhile pursuing this, I can prepare a draft patch for some of the uncovered typing issues so we can discuss more concretely.

[vyasr]

Overall I think you're probably right that this is more work than it's worth, and that it could lead to more complexity and hurdles in setting up the pre-commit environment than we would like (such as if we tried to install a GPU-enabled library like cupy).

From a 10000 foot view, I think the main benefit of doing these checks more is that they could uncover places where our internal dispatch patterns themselves are problematic. In cases where a function accepts an argument of type Any that needs to be dealt with differently depending on whether it's a pandas Series or a numpy array, the above style of if statements is probably necessary, but in other cases we may be able to use a single, simpler. ducktyped code path. However, in practice I think that's much more common when the objects being handled are our own objects (e.g. Series and DataFrame can often be handled identically rather than with separate branches) so I'm not sure how much the extra typing would actually buy us.

CC @shwina who's spent quite a bit of time with type hinting.

[github-actions]

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

[vyasr]

I think we should revisit this after #15190 and once cudf's internals become pure Python and designed around pylibcudf. Cython substantially complicates the issue here, as do the many design issues with our current internals. I suspect that we will be able to do a much better job with type hinting at that stage than we do now. At minimum we will be able to tighten up the typing of our internals (the user-facing objects are still tricky due to the complete flexibility of the pandas API).