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

API/DEPR: Remove +/- as setops for DatetimeIndex/PeriodIndex (GH9630) #14164

Merged
merged 3 commits into from
Sep 7, 2016
Merged

API/DEPR: Remove +/- as setops for DatetimeIndex/PeriodIndex (GH9630) #14164

merged 3 commits into from
Sep 7, 2016

Conversation

jorisvandenbossche
Copy link
Member

xref #13777, deprecations put in place in #9630

@jorisvandenbossche
Copy link
Member Author

jorisvandenbossche commented Sep 6, 2016 •
edited
Loading

Some todo's:

  • still have to further clean-up the tests a bit (due to the previous API the tests for set ops and numeric ops are a bit mixed)
  • need to add some more test cases for DTI - DTI (eg with NaNs)
  • for subtraction of two PeriodIndex objects, this is currently raising a TypeError, but I think we could actually allow it? Although that maybe depends on what is decided in ENH/API: Decide what to return Period - Period subtraction #13077

I think these should be the last remaining set ops as numeric ops that are now removed with this PR.

@jreback
Copy link
Contributor

jreback commented Sep 6, 2016

we need to wait for #13077 on Period-Period (so can discuss over there). Let's limit scope to the more useful DTI-DTI

jreback
jreback reviewed Sep 6, 2016
View reviewed changes
pandas/tseries/tests/test_base.py
rng3 = pd.date_range('1/1/2000', freq='D', periods=5, tz=tz)
other3 = pd.DatetimeIndex([], tz=tz)
expected3 = pd.date_range('1/1/2000', freq='D', periods=5, tz=tz)

Copy link
Contributor

Choose a reason for hiding this comment

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

test subtracting different lengths (ValueError) IIRC; this maybe already tested in another section though

Copy link
Contributor

Choose a reason for hiding this comment

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

mean this for below (dti-dti)

Copy link
Member Author

Choose a reason for hiding this comment

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

added a test for that (and added code to give this a nicer error message)

@jorisvandenbossche jorisvandenbossche added this to the 0.19.0rc milestone Sep 7, 2016
@codecov-io
Copy link

codecov-io commented Sep 7, 2016 •
edited
Loading

Current coverage is 85.25% (diff: 96.55%)

Merging #14164 into master will increase coverage by <.01%

@@             master     #14164   diff @@
==========================================
  Files           139        139          
  Lines         50496      50512    +16   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43049      43064    +15   
- Misses         7447       7448     +1   
  Partials          0          0

Powered by Codecov. Last update 8023029...21c6cec

jreback
jreback reviewed Sep 7, 2016
View reviewed changes
doc/source/whatsnew/v0.19.0.txt

Previous Behavior:
Previous behavior:
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to standardize here, I see @jorisvandenbossche and @sinhrks changing these!

IIRC I think we do more of Previous Behavior (e.g. capitalized), and american spelling (no Behaviour). But should just pick one :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, will do that in #14176

We indeed need to just make a choice between American or British :-)
(is capitalizing all words also an American habit? :-))

Copy link
Contributor

Choose a reason for hiding this comment

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

I think 2 capital words sets it off s bit more
using American spelling

Copy link
Member Author

Choose a reason for hiding this comment

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

while proofreading the whatsnew, I was thinking of putting the 'New/Old Behavior' in bold, that also sets it off a bit more

Copy link
Contributor

Choose a reason for hiding this comment

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

even better

@jreback
Copy link
Contributor

jreback commented Sep 7, 2016

lgtm.

@jorisvandenbossche jorisvandenbossche merged commit e88ad28 into pandas-dev:master Sep 7, 2016
@jorisvandenbossche jorisvandenbossche modified the milestones: 0.19.0rc, 0.19.0 Sep 7, 2016
trbs added a commit to trbs/pandas that referenced this pull request Sep 12, 2016
@trbs
Merge github.com:pydata/pandas into issue_9084_get_schema_index_param…
8ab1250 
…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 Sep 12, 2016
@trbs
Merge github.com:pydata/pandas into to_sql_secondary_indexes
a092198 
* 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)
  ...
@jorisvandenbossche jorisvandenbossche mentioned this pull request Sep 13, 2016
Closed
@jreback jreback mentioned this pull request Sep 13, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
API Design Clean
Projects
None yet
Milestone
0.19.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jorisvandenbossche @jreback @codecov-io @sinhrks