Skip to content

BUG-24408 Series.dt does not maintain own copy of index #24426

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

Merged
merged 6 commits into from
Dec 26, 2018

Conversation

JustinZhengBC
Copy link
Contributor

Previously, the dt accessor maintains its own copy of the index, so in-place operations would cause the indexes to be out of sync. This PR changes the accessor so it just gets the index from its parent.

@codecov
Copy link

codecov bot commented Dec 25, 2018

Codecov Report

Merging #24426 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24426      +/-   ##
==========================================
+ Coverage    92.3%    92.3%   +<.01%     
==========================================
  Files         163      163              
  Lines       51954    51953       -1     
==========================================
  Hits        47957    47957              
+ Misses       3997     3996       -1
Flag Coverage Δ
#multiple 90.71% <100%> (-0.01%) ⬇️
#single 42.99% <33.33%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/accessors.py 91% <100%> (-0.09%) ⬇️
pandas/util/testing.py 87.84% <0%> (+0.09%) ⬆️

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 2a09706...d21a893. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 25, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24426      +/-   ##
==========================================
- Coverage    92.3%    92.3%   -0.01%     
==========================================
  Files         163      163              
  Lines       51966    51965       -1     
==========================================
- Hits        47967    47966       -1     
  Misses       3999     3999
Flag Coverage Δ
#multiple 90.71% <100%> (-0.01%) ⬇️
#single 43% <33.33%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/accessors.py 91% <100%> (-0.09%) ⬇️

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 1905485...8326b9a. Read the comment docs.

@@ -485,6 +485,13 @@ def test_dt_accessor_invalid(self, ser):
ser.dt
assert not hasattr(ser, 'dt')

def test_dt_accessor_updates_on_inplace(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

can you test this for the other accessors as well. I think you can assert that the result.index is not the s.index to test.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can leave this test and add some in pandas/series/test_api.py

@jreback jreback added the Bug label Dec 25, 2018
@jreback jreback added this to the 0.24.0 milestone Dec 26, 2018
@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

lgtm. ping on green.

@JustinZhengBC
Copy link
Contributor Author

@jreback green

@jreback jreback merged commit ef1bd69 into pandas-dev:master Dec 26, 2018
@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

thanks!

thoo added a commit to thoo/pandas that referenced this pull request Dec 28, 2018
* upstream/master: (26 commits)
  DOC: Fixing doc upload (no such remote origin) (pandas-dev#24459)
  BLD: for C extension builds on mac, target macOS 10.9 where possible (pandas-dev#24274)
  POC: _eadata (pandas-dev#24394)
  DOC: Correct location (pandas-dev#24456)
  CI: Moving CircleCI build to Travis (pandas-dev#24449)
  BUG: Infer compression by default in read_fwf() (pandas-dev#22200)
  DOC: Fix minor typo in whatsnew (pandas-dev#24453)
  DOC: Add dateutil to intersphinx pandas-dev#24437 (pandas-dev#24443)
  DOC: Adding links to offset classes in timeseries.rst (pandas-dev#24448)
  DOC: Adding offsets to API ref (pandas-dev#24446)
  DOC: fix flake8 issue in groupby.rst (pandas-dev#24363)
  DOC: Fixing more doc warnings (pandas-dev#24438)
  API: Simplify repeat signature (pandas-dev#24447)
  BUG: to_datetime(Timestamp, utc=True) localizes to UTC (pandas-dev#24441)
  CLN: Cython Py2/3 Compatible Imports (pandas-dev#23940)
  DOC: Fixing more doc warnings (pandas-dev#24431)
  DOC: Removing old release.rst (pandas-dev#24427)
  BUG-24408 Series.dt does not maintain own copy of index (pandas-dev#24426)
  DOC: Fixing several doc warnings (pandas-dev#24430)
  ENH: fill_value argument for shift pandas-dev#15486 (pandas-dev#24128)
  ...
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Series.dt accessor throws exception after Series.drop when inplace=True
2 participants