Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore[python]: Turn on mypy strict mode #4415

Merged
merged 4 commits into from
Aug 15, 2022
Merged

Conversation

zundertj
Copy link
Collaborator

See the comment in pyproject.toml. It basically means we have not changed that much, as we still ignore the warning. But there is not really a feasible way until PyO3 supports generating type annotations. Otherwise, maintaining stubs would just be a huge waste of resources, as it is duplicating the "outer" python api.

Ticks the last box on the mypy list in #4044

See the comment in pyproject.toml. It basically means we have not changed that much, as we still ignore the warning. But there is not really a feasible way until PyO3 supports generating type annotations. Otherwise, maintaining stubs would just be a huge waste of resources, as it is duplicating the "outer" python api.
@github-actions github-actions bot added the python Related to Python Polars label Aug 14, 2022
@zundertj zundertj changed the title (chore)[python]: Turn on mypy strict mode chore[python]: Turn on mypy strict mode Aug 14, 2022
@stinodego
Copy link
Member

No need to add this to overrides, we can just do:

[tool.mypy]
strict = true
warn_return_any = false

I agree that manually adding stub files is overkill.

@zundertj
Copy link
Collaborator Author

That would override it for all modules, i.e. also numpy, pandas, etc. I would prefer to make the selection as small as possible.

@stinodego
Copy link
Member

stinodego commented Aug 14, 2022

That would override it for all modules, i.e. also numpy, pandas, etc. I would prefer to make the selection as small as possible.

You're right; I didn't think this through 😄

We still need the ignore imports override though. So you could add an additional section of overrides with just the Polars package and the warn_return_any in it.

EDIT: Actually, you can't ignore polars.polars in the overrides specifically. The mypy errors happen in the Python polars package. So that would mean that my suggestion was actually correct. What do you mean with this affecting numpy, pandas?

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2022

Codecov Report

Merging #4415 (b3dc257) into master (beb802a) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4415      +/-   ##
==========================================
+ Coverage   78.98%   79.01%   +0.02%     
==========================================
  Files         483      483              
  Lines       77094    77111      +17     
==========================================
+ Hits        60895    60926      +31     
+ Misses      16199    16185      -14     
Impacted Files Coverage Δ
...polars-time/src/chunkedarray/rolling_window/mod.rs 71.53% <0.00%> (-0.77%) ⬇️
polars/polars-lazy/src/logical_plan/alp.rs 84.39% <0.00%> (-0.73%) ⬇️
py-polars/polars/testing.py 94.14% <0.00%> (-0.54%) ⬇️
...olars/polars-core/src/frame/groupby/into_groups.rs 58.12% <0.00%> (-0.28%) ⬇️
polars/polars-lazy/src/physical_plan/planner/lp.rs 88.11% <0.00%> (+0.02%) ⬆️
polars/polars-lazy/src/logical_plan/conversion.rs 78.34% <0.00%> (+0.12%) ⬆️
polars/polars-core/src/series/mod.rs 64.20% <0.00%> (+0.15%) ⬆️
polars/polars-core/src/fmt.rs 73.07% <0.00%> (+0.18%) ⬆️
polars/polars-core/src/datatypes/mod.rs 72.40% <0.00%> (+0.18%) ⬆️
...ars-arrow/src/kernels/rolling/no_nulls/variance.rs 88.55% <0.00%> (+0.23%) ⬆️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

py-polars/pyproject.toml Outdated Show resolved Hide resolved
@zundertj
Copy link
Collaborator Author

Python 3.7 test is fixed in #4416.

@stinodego
Copy link
Member

I think you forgot to push your latest changes removing fsspec and pyarrow from the overrides. The Python test fix has been merged, so with a rebase + push I think we're good to go!

Copy link
Member

@stinodego stinodego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 🚀

@ritchie46
Copy link
Member

That's quite an achievement. Well done. 👍

@ritchie46 ritchie46 merged commit 4d40021 into pola-rs:master Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants