Typing/mypy settings

[m-richards]

Supporting mypy changes to experiment with resolving mypy issues in #1905.

I've just initially pushed now with failing tests to show a point, but I'll probably drive discussion from the other PR for now.

[codecov]

Codecov Report

Attention: Patch coverage is 96.72131% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.52%. Comparing base (812b2a8) to head (bb1abb8).
Report is 199 commits behind head on main.

Files with missing lines	Patch %	Lines
pandera/engines/polars_engine.py	94.44%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1911      +/-   ##
==========================================
- Coverage   94.28%   93.52%   -0.76%     
==========================================
  Files          91      121      +30     
  Lines        7013     9395    +2382     
==========================================
+ Hits         6612     8787    +2175     
- Misses        401      608     +207     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[m-richards]

Hey @cosmicBboy, I was wondering if you might be able to share some background on how pandera/api/polars/types.py::PolarsData came to be ( I can see it was added in #1373, and before that, d9873bd). If you don't recall then no worries.

Mypy does not like this very much due to key being an optional attribute, but from what I understand the usages are semantically different, key is provided to apply checks to a specific column, and when it is none, it seems to be used for coercion of schemas with a single data type. There seem to be a set of cases where a lazyframe is wrapped in PolarsData and subsequently unwrapped, I was going to have a go at simplifying this, but wanted to see if that was a bad idea before I got too far.

(I would also understand if you're not too concerned with getting mypy to behave on library internals, so happy to just leave that out if you prefer)

[cosmicBboy]

I was wondering if you might be able to share some background on how pandera/api/polars/types.py::PolarsData came to be ( I can see it was added in #1373, and before that, d9873bd). If you don't recall then no worries.

hey @m-richards apologies for the late reply here. So PolarsData came about because I wanted check functions to only take a single argument so that it can operate both on an entire dataframe or on a single column of the dataframe. The key is optional because if it's not None, the assumption is that the key is used to apply a check to a specific column in the dataframe, if it's None, then the check is applied to the entire dataframe (e.g. Check.gt(0)) should be applicable to an entire dataframe.

This is needed for dataframe libraries like polars and pyspark where it's better to construct the validation rules using the lazily constructed computation graph.

Mypy does not like this very much due to key being an optional attribute

What are the issues here specifically? Seems odd that mypy wouldn't support Optional[TYPE].

I would also understand if you're not too concerned with getting mypy to behave on library internals, so happy to just leave that out if you prefer

I don't think it's very practical to change the check function signature: I think it keeps the Check function signature clean, simple, and easy to understand (open to debating this point!)

I always support getting mypy to behave with library internals unless it involves contorting the code a lot just to make mypy happy.

[cosmicBboy]

hey @m-richards looked into the PolarsData type issue: #1972

This PR changes the signature to key: str, let me know if this handles the mypy issues you're seeing.

[m-richards]

hey @m-richards looked into the PolarsData type issue: #1972

This PR changes the signature to key: str, let me know if this handles the mypy issues you're seeing.

@cosmicBboy I'll answer both replies in one, since they're linked:

What are the issues here specifically? Seems odd that mypy wouldn't support Optional[TYPE].

Yeah I should have been more specific. mypy supports it fine, the problem comes is that e.g. in pandera/backends/polars/builtin_checks.py::in_range
there's

def in_range(
    data: PolarsData,
    min_value: T,
    max_value: T,
    include_min: bool = True,
    include_max: bool = True,
) -> pl.LazyFrame:
    col = pl.col(data.key) 

which gives
error: Argument 1 to "__call__" of "Col" has incompatible type "str | None"; expected "str | ...
in - practice you know / and have tested such that the PolarsData in this case is never going to have data.key is None at this call site. I wasn't sure if we could just change the representation of all columns from None to "*" so here (and in the local copy of this branch I have) I was seeing what it would look like if you treat these as two seperate PolarsData sub-types for the sake of mypy - the "all columns" case and the "specific column" case.

Agree that a simpler signature is nicer and easier to think about and it seems that #1972 handles this from a mypy perspective (47 errors instead of 62
running mypy with these lines

; [mypy-pandera.engines.polars_engine]
; ignore_errors = True
;
; [mypy-pandera.backends.polars.builtin_checks]
; ignore_errors = True

commented in the mypy config.