_concat_rangeindex_... now returns an empty RangeIndex for empty ranges #18214
Conversation
… empty ranges; Overall the code is more readable by adding the constructors at the beginning and explicitly calling `RangeIndex` constructor. One range index test needed to be changed, because concatenating empty range indexes leads to an empty integer index from now on;
pandas/core/dtypes/concat.py
Outdated
| stop = obj._stop | ||
|
|
||
| return indexes[0].__class__(start, stop, step) | ||
| # In this case return an empty integer index. |
There was a problem hiding this comment.
I think that this should be RangeIndex(0, 1) to be consistent with our default indices, e.g. Series([]).index
There was a problem hiding this comment.
RangeIndex(0, 1) or rather RangeIndex(0, 0)?
There was a problem hiding this comment.
it is changed to empty range
jreback
added
Clean
Compat
|
Hello @SmokinCaterpillar! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on November 11, 2017 at 11:32 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #18214 +/- ##
==========================================
- Coverage 91.42% 91.41% -0.02%
==========================================
Files 163 163
Lines 50068 50064 -4
==========================================
- Hits 45777 45764 -13
- Misses 4291 4300 +9
Continue to review full report at Codecov.
|
jreback
changed the title
_concat_rangeindex_... now returns an empty Int64Index for empty ranges
pandas/core/dtypes/concat.py
Outdated
| @@ -604,12 +603,8 @@ def _concat_rangeindex_same_dtype(indexes): | |||
| # Get the stop value from "next" or alternatively | |||
| # from the last non-empty index | |||
| stop = non_empty_indexes[-1]._stop if next is None else next | |||
| return RangeIndex(start, stop, step) | |||
| else: | |||
There was a problem hiding this comment.
maybe remove the else here (and just return the RangeIndex)
There was a problem hiding this comment.
ok, I personally like the else in such situations, it's more explicit to me, I guess it's a matter of taste. I'll remove it :-)
|
thanks! |
This is a follow up to PR #18191.
Overall the code is more readable by adding the constructors at the beginning and
explicitly calling
RangeIndexconstructor.One range index test was changed to accommodate for the new behavior of the index concatenation function.
git diff upstream/master -u -- "*.py" | flake8 --diff