-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
DEPR: remove v018 resample compatibilitiy #20782
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20782 +/- ##
==========================================
- Coverage 91.84% 91.78% -0.07%
==========================================
Files 153 153
Lines 49305 49247 -58
==========================================
- Hits 45286 45203 -83
- Misses 4019 4044 +25
Continue to review full report at Codecov.
|
@jorisvandenbossche @TomAugspurger if any comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general, just a small note on something I don't fully understand
@jorisvandenbossche I don't see a note from you. Perhaps it wasn't submitted? |
@@ -62,20 +62,6 @@ class Resampler(_GroupBy): | |||
_attributes = ['freq', 'axis', 'closed', 'label', 'convention', | |||
'loffset', 'base', 'kind'] | |||
|
|||
# API compat of allowed attributes | |||
_deprecated_valids = _attributes + ['__doc__', '_cache', '_attributes', | |||
'binner', 'grouper', 'groupby', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops, didn't push the button probably :-)
My question was about the above _deprecated_valids
: what was the purpose of them? At least before, those attributes did not raise any warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you can probably ignore my comment, those attributes are "hided" from tab completion anyway, and didn't change for the rest. I suppose this was here before to prevent raising any warning on them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this was gymnastics to make all of this work
closes #20554