Correctly concatenate pandas.Index objects #875
Conversation
We now have a dedicated method, Coordinate.concat that will correctly concatenate pandas.Index objects, even if they can't be properly expressed as NumPy arrays (e.g., PeriodIndex and MultiIndex). As part of this change, I removed and replaced the internal `interleaved_concat` routine. It turns out we can do this with an inverse permutation instead, which results in much simpler and cleaner code. In particular, we no longer need a special path to support dask.array. This should help with GH818.
| @@ -66,6 +67,52 @@ def _dummy_copy(xarray_obj): | |||
| raise AssertionError | |||
| return res | |||
|
|
|||
| def _is_one_or_none(obj): | |||
| return obj == 1 or obj is None | |||
There was a problem hiding this comment.
do you want to treat True as 1? If not, this should be obj is 1 or obj is None.
There was a problem hiding this comment.
Interesting! I never knew that.
I always thought ints & strings should use ==, but for -1 to 256 is is fine: http://stackoverflow.com/questions/2239737/is-it-better-to-use-is-or-for-number-comparison-in-python
There was a problem hiding this comment.
But this depends on the interpreter, then. CPython might work, but others might not I guess?
There was a problem hiding this comment.
These slice objects are generated internally by groupby, so I think it's pretty far fetched to get step is True. Using == feels more idiomatic, anyways.
| along the given dimension. | ||
| """ | ||
| variables = list(variables) | ||
| print(variables) |
There was a problem hiding this comment.
guessing this is left over from debugging
We now have a dedicated method, Coordinate.concat that will correctly
concatenate pandas.Index objects, even if they can't be properly expressed as
NumPy arrays (e.g., PeriodIndex and MultiIndex).
As part of this change, I removed and replaced the internal
interleaved_concatroutine. It turns out we can do this with an inverse permutation instead, which
results in much simpler and cleaner code. In particular, we no longer need a
special path to support dask.array.
This should help with #818.