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.

Sign up for GitHub

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

Jump to bottom

Fix type annotations in pandas.core.resample #26398

Merged
merged 2 commits into from
May 15, 2019
Merged

Fix type annotations in pandas.core.resample #26398

merged 2 commits into from
May 15, 2019

Conversation

gwrome
Copy link
Contributor

@gwrome gwrome commented May 14, 2019

Part of #25882

  • tests passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

This PR addresses the following mypy errors in pandas.core.resample:

pandas/core/resample.py:34: error: Need type annotation for '_shared_docs_kwargs'
pandas/core/resample.py:876: error: All conditional function variants must have identical signatures
pandas/core/resample.py:884: error: All conditional function variants must have identical signatures
pandas/core/resample.py:891: error: All conditional function variants must have identical signatures
pandas/core/resample.py:1101: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "DatetimeIndexResampler"
pandas/core/resample.py:1101: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "Resampler"
pandas/core/resample.py:1101: error: Definition of "_downsample" in base class "_GroupByMixin" is incompatible with definition in base class "DatetimeIndexResampler"
pandas/core/resample.py:1101: error: Definition of "_groupby_and_aggregate" in base class "_GroupByMixin" is incompatible with definition in base class "Resampler"
pandas/core/resample.py:1217: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "PeriodIndexResampler"
pandas/core/resample.py:1217: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "DatetimeIndexResampler"
pandas/core/resample.py:1217: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "Resampler"
pandas/core/resample.py:1217: error: Definition of "_downsample" in base class "_GroupByMixin" is incompatible with definition in base class "PeriodIndexResampler"
pandas/core/resample.py:1217: error: Definition of "_downsample" in base class "_GroupByMixin" is incompatible with definition in base class "DatetimeIndexResampler"
pandas/core/resample.py:1217: error: Definition of "_groupby_and_aggregate" in base class "_GroupByMixin" is incompatible with definition in base class "Resampler"
pandas/core/resample.py:1247: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "DatetimeIndexResampler"
pandas/core/resample.py:1247: error: Definition of "_upsample" in base class "_GroupByMixin" is incompatible with definition in base class "Resampler"
pandas/core/resample.py:1247: error: Definition of "_downsample" in base class "_GroupByMixin" is incompatible with definition in base class "DatetimeIndexResampler"
pandas/core/resample.py:1247: error: Definition of "_groupby_and_aggregate" in base class "_GroupByMixin" is incompatible with definition in base class "Resampler"
pandas/core/resample.py:1299: error: Incompatible types in assignment (expression has type "Tuple[str, ...]", base class "Grouper" defined the type as "Tuple[str, str, str, str, str]")

gwrome
gwrome commented May 14, 2019
View reviewed changes
pandas/core/resample.py
@@ -964,6 +965,7 @@ def __init__(self, obj, *args, **kwargs):
self._groupby.grouper.mutated = True
self.groupby = copy.copy(parent.groupby)

@no_type_check
Copy link
Contributor Author

@gwrome gwrome May 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This file has a set of mismatched signatures that's much more involved to bring into alignment that the ones we did in other PRs.

_upsample

DatetimeIndexResampler and Resampler both define an _upsample method. The two signatures are only marginally different and could be harmonized easily. But _GroupByMixin defines an _apply method that is significantly different and then assigns that method to the class's _upsample variable (as well as _downsample and _groupby_and_aggregate).

_downsample

_downsample has similar relationships between PeriodIndexResampler, DatetimeIndexResampler, and _GroupByMixin`.

_groupby_and_aggregate

_groupby_and_aggregate has similar relationships between Resampler and _GroupByMixin.

Some parts of the _apply signature appear to be unused in this file. I'm investigating how much mucking around I can do with them. It might be possible to bring them all in alignment.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gwrome there is lots of magic to make grouby/resample/rolling play nicely. But can of course do this another time.

@codecov
Copy link

codecov bot commented May 14, 2019

Codecov Report

Merging #26398 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26398      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         174      174              
  Lines       50749    50752       +3     
==========================================
- Hits        46534    46532       -2     
- Misses       4215     4220       +5
Flag Coverage Δ
#multiple 90.19% <100%> (ø) ⬆️
#single 41.17% <100%> (-0.15%) ⬇️
Impacted Files Coverage Δ
pandas/core/resample.py 97.23% <100%> (ø) ⬆️
pandas/core/groupby/grouper.py 98.53% <100%> (ø) ⬆️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97.02% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.6% <0%> (-0.11%) ⬇️

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 e5d15b2...1abd099. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented May 14, 2019 •
edited
Loading

Codecov Report

Merging #26398 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26398      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         174      174              
  Lines       50749    50752       +3     
==========================================
- Hits        46534    46532       -2     
- Misses       4215     4220       +5
Flag Coverage Δ
#multiple 90.19% <100%> (ø) ⬆️
#single 41.17% <100%> (-0.15%) ⬇️
Impacted Files Coverage Δ
pandas/core/resample.py 97.23% <100%> (ø) ⬆️
pandas/core/groupby/grouper.py 98.53% <100%> (ø) ⬆️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97.02% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.6% <0%> (-0.11%) ⬇️

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 e5d15b2...1abd099. Read the comment docs.

@gfyoung gfyoung added Resample resample method Typing type annotations, mypy/pyright type checking labels May 15, 2019
@jreback jreback added this to the 0.25.0 milestone May 15, 2019
@jreback jreback merged commit 0382f20 into pandas-dev:master May 15, 2019
@jreback
Copy link
Contributor

jreback commented May 15, 2019

thanks

@gwrome gwrome deleted the blacklist-resample branch May 16, 2019 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

Assignees
No one assigned
Labels
Resample resample method Typing type annotations, mypy/pyright type checking
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.
3 participants
@gwrome @jreback @gfyoung