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

Set no_implicit_reexport = true in pyproject.toml #4211

Merged
merged 4 commits into from
Aug 3, 2022

Conversation

matteosantama
Copy link
Contributor

One step closer to enabling strict = true for mypy.

@github-actions github-actions bot added the python Related to Python Polars label Aug 1, 2022
@matteosantama matteosantama changed the title No implicit reexport v1 Set no_implicit_reexport = true in pyproject.toml Aug 1, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2022

Codecov Report

Merging #4211 (d695639) into master (95074d9) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4211      +/-   ##
==========================================
- Coverage   64.17%   64.11%   -0.06%     
==========================================
  Files         457      457              
  Lines       75611    75613       +2     
==========================================
- Hits        48521    48483      -38     
- Misses      27090    27130      +40     
Impacted Files Coverage Δ
py-polars/polars/internals/frame.py 91.55% <ø> (ø)
py-polars/polars/__init__.py 100.00% <100.00%> (ø)
py-polars/polars/internals/__init__.py 100.00% <100.00%> (ø)
...s/polars-core/src/chunked_array/ops/unique/rank.rs 40.48% <0.00%> (-8.31%) ⬇️
...s/polars-core/src/chunked_array/upstream_traits.rs 69.34% <0.00%> (-2.58%) ⬇️
...olars-lazy/src/physical_plan/executors/scan/csv.rs 96.15% <0.00%> (-1.93%) ⬇️
...lars/polars-core/src/chunked_array/builder/list.rs 62.56% <0.00%> (-1.12%) ⬇️
polars/polars-core/src/series/series_trait.rs 12.28% <0.00%> (-1.06%) ⬇️
...ars/polars-core/src/chunked_array/ops/any_value.rs 77.39% <0.00%> (-0.87%) ⬇️
...olars/polars-core/src/frame/groupby/into_groups.rs 59.88% <0.00%> (-0.30%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95074d9...d695639. Read the comment docs.

@matteosantama
Copy link
Contributor Author

I should link #4044

@matteosantama matteosantama force-pushed the no-implicit-reexport-v1 branch from b5d6d80 to d695639 Compare August 1, 2022 18:13
@ritchie46 ritchie46 requested review from zundertj and stinodego August 2, 2022 06:47
@@ -28,3 +28,34 @@
)
from .series import Series, wrap_s
from .whenthen import when # used in expr.clip()

__all__ = [
Copy link
Member

@stinodego stinodego Aug 2, 2022

Choose a reason for hiding this comment

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

Why not include argsort_by, date_range, and _deser_and_exec in this list? These are now imported but not available through __all__.

Same for the other __init__ file, there are a few instances missing from __all__, like Null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only because mypy didn't complain about them, meaning they are not imported from anywhere else in the codebase. It might better to remove them as imports from the __init__ than to add them to __all__. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

You're right, let's keep that for a different PR!

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.

Looks good to me!

I mentioned before that I want to avoid using typing.cast if at all possible, but this seems like a valid use case. Nice work.

@ritchie46 ritchie46 merged commit c745df1 into pola-rs:master Aug 3, 2022
@matteosantama matteosantama deleted the no-implicit-reexport-v1 branch August 3, 2022 11:16
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