API/DEPR: Remove +/- as setops for Index (GH8227) by jorisvandenbossche · Pull Request #14127 · pandas-dev/pandas (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
xref #13777, deprecations put in place in #8227
- tests added / passed
- passes
git diff upstream/master | flake8 --diff - whatsnew entry
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not defined in super? so this should raise NotImplementedError I think?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, this is something that I still have to handle.
The problem is that sub/add are added for subclasses in either _add_numericlike_set_methods_disabled or _add_numeric_methods or both ovenwritten (always handled both), but in the base class we only want __add__ and not __sub__
If I leave it in here, I think it should rather raise a TypeError? (for usage of base Index class)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah TypeError is prob ok too
the only reason to raise NotImplemmeted is that then subclasses could override - but we don't want that
Current coverage is 85.25% (diff: 100%)
@@ master #14127 diff @@
Files 139 139
Lines 50501 50495 -6
Methods 0 0
Messages 0 0
Branches 0 0
- Hits 43058 43051 -7
- Misses 7443 7444 +1
Partials 0 0
Powered by Codecov. Last update 4488f18...dbcc655
@jreback yep, as it is quite a substantial change (for people that ignored the warnings)
Is there a reason that we do not allow numeric ops for the base Index class? (eg pd.Index(['a', 'b'] could have a clear meaning, just letting the objects determine whether an operation can be handled)
Because eg the __add__ in the Index class could benefit from the machinery in _add_numeric_methods_binary (eg to ensure attributes like name are passed correctly. Something that is also not correct on current master)
I think we don't allow numeric operations on Index as only possible one is add for string concat
nothing else makes sense
in general these r not well defined so no reason to allow
I think we should let the objects in the Index decide whether the numeric operation is valid or not. This is the approach NumPy uses for object dtype. For example, one might put decimal objects in a pandas.Index.
the numpy approach leads to lots of bugs and edge cases - constraining things somewhat to not allowing everything on everything is much more user friendly
decimal objects in an Index are a disaster - should not be encouraged
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be more specific that 'when possible' (maybe say for numeric & datetimelike)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datetimelike not yet (DatetimeIndex - DatetimeIndex does still set op, but this one is a bit more work to remove).
And + also works for strings, so not only numeric. Further, for numeric indices, + and - did already do numeric ops and not setops .. (to make it a bit more complex :-))
So I did not directly come up with a more accurate but simple wording, but if you have a suggestion all ears!
[WIP] API/DEPR: Remove +/- as setops for Index (GH8227) API/DEPR: Remove +/- as setops for Index (GH8227)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this clear that this ONLY applies to Index itself and not-subclasses (maybe show that numeric/datetimelike ops are unchanged)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, will try to clarify
trbs added a commit to trbs/pandas that referenced this pull request
…eter
- github.com:pydata/pandas: (554 commits) BUG: compat with Stata ver 111 Fix: F999 dictionary key '2000q4' repeated with different values (pandas-dev#14198) BLD: Test for Python 3.5 with C locale BUG: DatetimeTZBlock can't assign values near dst boundary BUG: union_categorical with Series and cat idx BUG: fix str.contains for series containing only nan values BUG: Categorical constructor not idempotent with ext dtype TST: Make encoded sep check more locale sensitive (pandas-dev#14161) DOC: minor typo in 0.19.0 whatsnew file (pandas-dev#14185) BUG: fix tz-aware datetime convert to DatetimeIndex (GH 14088) BUG : bug in setting a slice of a Series with a np.timedelta64 RLS: v0.19.0rc1 DOC: clean-up 0.19.0 whatsnew file (pandas-dev#14176) DOC: cleanup build warnings (pandas-dev#14172) Add steps to run gbq integration testing to the contributing docs (pandas-dev#14144) ENH: concat and append now can handle unordered categories (pandas-dev#13767) DEPR: Deprecate pandas.core.datetools (pandas-dev#14105) API/DEPR: Remove +/- as setops for DatetimeIndex/PeriodIndex (GH9630) (pandas-dev#14164) Fix trivial typo in comment (pandas-dev#14174) API/DEPR: Remove +/- as setops for Index (GH8227) (pandas-dev#14127) ...
trbs added a commit to trbs/pandas that referenced this pull request
- github.com:pydata/pandas: (554 commits) BUG: compat with Stata ver 111 Fix: F999 dictionary key '2000q4' repeated with different values (pandas-dev#14198) BLD: Test for Python 3.5 with C locale BUG: DatetimeTZBlock can't assign values near dst boundary BUG: union_categorical with Series and cat idx BUG: fix str.contains for series containing only nan values BUG: Categorical constructor not idempotent with ext dtype TST: Make encoded sep check more locale sensitive (pandas-dev#14161) DOC: minor typo in 0.19.0 whatsnew file (pandas-dev#14185) BUG: fix tz-aware datetime convert to DatetimeIndex (GH 14088) BUG : bug in setting a slice of a Series with a np.timedelta64 RLS: v0.19.0rc1 DOC: clean-up 0.19.0 whatsnew file (pandas-dev#14176) DOC: cleanup build warnings (pandas-dev#14172) Add steps to run gbq integration testing to the contributing docs (pandas-dev#14144) ENH: concat and append now can handle unordered categories (pandas-dev#13767) DEPR: Deprecate pandas.core.datetools (pandas-dev#14105) API/DEPR: Remove +/- as setops for DatetimeIndex/PeriodIndex (GH9630) (pandas-dev#14164) Fix trivial typo in comment (pandas-dev#14174) API/DEPR: Remove +/- as setops for Index (GH8227) (pandas-dev#14127) ...