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.

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

ENH: Make RangeIndex.append() return RangeIndex when possible #16213

Merged
merged 1 commit into from
May 12, 2017

Conversation

toobaz
Copy link
Member

@toobaz toobaz commented May 3, 2017

This is analogous to what is already done for RangeIndex.union(), but there doesn't seem to be much scope for code reuse (what could be, in principle, reused in .append of all Index subclasses is the initial casting to list/name handling, I could do in a separate PR).

@codecov
Copy link

codecov bot commented May 3, 2017

Codecov Report

Merging #16213 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16213      +/-   ##
==========================================
- Coverage   90.89%   90.87%   -0.02%     
==========================================
  Files         162      162              
  Lines       50884    50915      +31     
==========================================
+ Hits        46250    46269      +19     
- Misses       4634     4646      +12
Flag Coverage Δ
#multiple 88.66% <100%> (-0.02%) ⬇️
#single 40.32% <58.06%> (+0.01%) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/range.py 92.81% <100%> (+0.7%) ⬆️
pandas/plotting/_converter.py 63.54% <0%> (-1.82%) ⬇️

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 f154966...a77ee00. Read the comment docs.

@codecov
Copy link

codecov bot commented May 3, 2017

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16213      +/-   ##
==========================================
- Coverage   90.35%   90.34%   -0.01%     
==========================================
  Files         161      161              
  Lines       50866    50897      +31     
==========================================
+ Hits        45962    45985      +23     
- Misses       4904     4912       +8
Flag Coverage Δ
#multiple 88.13% <100%> (ø) ⬆️
#single 40.19% <58.06%> (-0.1%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/range.py 92.81% <100%> (+0.7%) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.59% <0%> (-0.1%) ⬇️
pandas/core/common.py 91.03% <0%> (+0.34%) ⬆️

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 94ef7b6...89da68a. Read the comment docs.

@@ -508,6 +508,7 @@ Other Enhancements
- Compatability with Jupyter notebook 5.0; MultiIndex column labels are left-aligned and MultiIndex row-labels are top-aligned (:issue:`15379`)

- ``TimedeltaIndex`` now has a custom datetick formatter specifically designed for nanosecond level precision (:issue:`8711`)
- ``RangeIndex.append`` now returns a ``RangeIndex`` object when possible (:issue:`16212`)
Copy link
Contributor

Choose a reason for hiding this comment

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

move to 0.20.1

"""
Append a collection of Index options together

Parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, yeah any way to share some code?

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, more or less ready, but it involves many more changes/files and probably some discussion, if it's OK I will open another PR immediately after this

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 simply put up another PR which bases on this one.

Copy link
Member Author

@toobaz toobaz May 3, 2017

Choose a reason for hiding this comment

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

I have to fix one error, then I will

Copy link
Member Author

Choose a reason for hiding this comment

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

see #16236

@jreback jreback added Enhancement Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels May 3, 2017
@@ -18,6 +18,7 @@ Highlights include:

Enhancements
~~~~~~~~~~~~
- ``RangeIndex.append`` now returns a ``RangeIndex`` object when possible (:issue:`16212`)
Copy link
Contributor

Choose a reason for hiding this comment

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

move to 0.21.0

@toobaz toobaz force-pushed the range_append branch 2 times, most recently from 4f8762d to 7b3cecb Compare May 4, 2017 14:19
@toobaz
Copy link
Member Author

toobaz commented May 4, 2017

The last push just makes few lines of the tests nicer

@toobaz toobaz mentioned this pull request May 4, 2017
2 tasks
@toobaz
Copy link
Member Author

toobaz commented May 12, 2017

@jreback as I had anticipated, #16236 involves some non-trivial discussion (and additional work which I won't be able to do in the next days). For the moment, I invite you to reconsider this PR, which should be straightforward: after, it will be easier to focus on that pure refactoring PR.

@@ -941,3 +941,38 @@ def test_where_array_like(self):
for klass in klasses:
result = i.where(klass(cond))
tm.assert_index_equal(result, expected)

def test_append(self):
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the issue number as comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

(done)

@jreback jreback added this to the 0.21.0 milestone May 12, 2017
@jreback
Copy link
Contributor

jreback commented May 12, 2017

ok, fine to merge. ping on green.

@toobaz
Copy link
Member Author

toobaz commented May 12, 2017

ping

@jreback jreback merged commit 50e95e0 into pandas-dev:master May 12, 2017
@jreback
Copy link
Contributor

jreback commented May 12, 2017

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RangeIndex.append() should return RangeIndex when possible
3 participants