BUG: union should not try to sort inplace because platform impls differ as to when sorting occurs for objects that cannot be compared
State: closed

closes #5039


Labels:
name: Bug
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
@jtratner Can you try this out? Thanks

running a vbench ... likely a hit since this will now copy

Actually, nothing huge:

```
mask_bools                                   |  14.6003 |  13.2553 |   1.1015 |
reindex_fillna_backfill_float32              |   0.4900 |   0.4440 |   1.1035 |
timeseries_sort_index                        |  21.4117 |  19.3573 |   1.1061 |
join_dataframe_integer_2key                  |   5.2234 |   4.7040 |   1.1104 |
stat_ops_frame_mean_int_axis_0               |   0.6540 |   0.5771 |   1.1333 |
stat_ops_frame_sum_int_axis_0                |   0.4297 |   0.3790 |   1.1338 |
reindex_multiindex                           |   1.6460 |   1.4460 |   1.1383 |
read_store_table_panel                       |  22.6684 |  19.8513 |   1.1419 |
stat_ops_frame_sum_float_axis_1              |   0.7653 |   0.6527 |   1.1725 |
groupby_simple_compress_timing               |  26.6473 |  22.1800 |   1.2014 |
-------------------------------------------------------------------------------
Test name                                    | head[ms] | base[ms] |  ratio   |
-------------------------------------------------------------------------------
```

Why not just sort on id if it can't sort on anything else? I.e.:

`result = result[np.argsort(map(id, arr))]`

That way you don't have to copy for an edge case.

It has to be dtype object already.

I guess that's not necessarily stable/repeatable tho

and it will copy on that kind of slice

Definitely - I was just thinking you would put that in the except clause with TypeErrors so you're generally _not_ copying where possible.

But sorting by id is bad anyways, since it's not really deterministic between calls with non-interned values.

But this does appear to fix the issue. :smile: 

the whole idea of catching a failed inplace sort is botched

wait, no never mind. popped up again on a different thing now.

arg we need mac testing

can u put up the error

Is there something in the test suite that's dependent upon randomness or order of instantiation? Weirdly it passes with `nosetests`, but fails if I do `nosetests --with-progressive`  Tried this on multiple different runs.

``` python
FAIL: pandas.computation.tests.test_eval:TestAlignment.test_basic_series_frame_alignment('numexpr', 'python')
  /usr/local/bin/vim +872 pandas/computation/tests/test_eval.py  # check_basic_series_frame_alignment
    testit(r_idx_type, c_idx_type, index_name)
  /usr/local/bin/vim +867 pandas/computation/tests/test_eval.py  # testit
    assert_frame_equal(res, expected)
  /usr/local/bin/vim +484 pandas/util/testing.py  # assert_frame_equal
    assert_index_equal(left.columns, right.columns)
  /usr/local/bin/vim +364 pandas/util/testing.py  # assert_index_equal
    right.dtype))
AssertionError: [index] left [object Index([u'98ןן2הנ50פ', 2000-01-03 00:00:00, 2000-01-04 00:00:00, 2000-01-05 00:00:00, 2000-01-06 00:00:00, 2000-01-07 00:00:00, u'אוז91כיע5צ', u'י09לירג6אש', u'יא0ק0פלר31', u'ם8327יכסצנ', u'ןעץמףרן43ץ', u'קזהאצ5כצון'], dtype='object')], right [Index([u'98ןן2הנ50פ', 2000-01-04 00:00:00, 2000-01-05 00:00:00, 2000-01-06 00:00:00, u'י09לירג6אש', 2000-01-03 00:00:00, u'אוז91כיע5צ', u'יא0ק0פלר31', 2000-01-07 00:00:00, u'ם8327יכסצנ', u'ןעץמףרן43ץ', u'קזהאצ5כצון'], dtype='object') object]
```

let me see ... 

Okay, now I have figured it out - it fails if you run it with "-A 'not network and not slow'". must be something to do with test ordering.

that repr is screwed up too

is that just a mistype ? or are those two consecutive commas a bug?

Nope, that's wrong too, it just intermittently fails now - very confusing...

I wonder if you bumped up the size of the objects it's generating - I bet you'd see it on your computer (double or triple them just for testing)

sweet i now get the runtimewarning

which of course makes sense

I can't reproduce your most recent error.

It only occurs about 60% of the time for me - I finally figured out that it was occuring semi-randomly by running it endlessly:

``` python
from subprocess import call
[call(['nosetests', 'pandas.computation.tests.test_eval:TestAlignment.test_basic_series_frame_alignment']) for i in range(30)]
```

@cpcloud what is this test even supposed to test?  `s + df` just ends up with a big NaN frame because it checks columns before Index, a portion like this:

```
            2000-01-11 00:00:00  טרם7חעחש4ק  איט8צ7ךןהד  ה1קרט2ךץמל  \
R0
איט8צ7ךןהד                  NaN         NaN         NaN         NaN
נצ6מנ4םדץע                  NaN         NaN         NaN         NaN
ה1קרט2ךץמל                  NaN         NaN         NaN         NaN
טרם7חעחש4ק                  NaN         NaN         NaN         NaN
יכלפדה2ערם                  NaN         NaN         NaN         NaN
הועפץג78ןר                  NaN         NaN         NaN         NaN
בעזלו2ףננק                  NaN         NaN         NaN         NaN
ענמ3ך8שז1ג                  NaN         NaN         NaN         NaN
אנ0ףפד38כ5                  NaN         NaN         NaN         NaN
כע13רץןהאר                  NaN         NaN         NaN         NaN
```

@cpcloud how about this for union/sorting generally:
1. Catch the TypeError
2. Warn that the sort order is undefined with unsortable objects
3. Return whatever resulted.

And then in the test suite, don't bother comparing the result if the index and column types aren't compatible. There's no point in spending a lot of time trying to test undefined behavior and figuring out why some routes produce different, still-undefined behavior. 

So I think this would just be checking that column and index are either equal or are an integer/float combo.

@jtratner 

> what is this test even supposed to test?

Just that the operation in `pd.eval` space returns the same result as the operation in plain Python space, nothing more specific than that.

> don't bother comparing the result if the index and column types aren't compatible

There's only a "common sense" notion of "index and column types that aren't compatible" since indices are converted to object dtype if they are not the same dtype. The idea for these tests was just to make sure that operations that _don't_ make sense do the same thing no matter how you're evaluating an expression.

I suspect the reason they are happening differently is because Python space uses `combine_*` and friends (which may join indices differently with the index) while `eval` does a `join` on each axis (if the indices differ on a given axis), reindexes each axis by the new index, passes the expression with the underlying `numpy` arrays to `numexpr`, then reconstructs the result. The time complexity of this "index alignment" operation is `O(# of terms * max(ndim for each term))`

This _is_ an actual issue because the result should be the same (it is on Linux AFAICT) but Mac OS seems to do something a bit different with joining as opposed to combining objects with `combine_*` and friends.

Does that all make sense? 

@jtratner Can you show me the output of the following:

``` python
import numpy as np
import pandas as pd
from pandas.util.testing import makeCustomDataframe as mkdf

f = lambda *args, **kwargs: np.random.randn()
df = mkdf(10, 7, data_gen_f=f, r_idx_type='i', c_idx_type='dt')
s = pd.Series(np.random.randn(5), df.index[:5])
x, y = df.align(s, axis=1)
print(y)
```

That totally makes sense - - I was just thinking that there isn't really a
'correct' order for these objects since they are unorderable. Plus I
figured if we could avoid a copy in the general case that would be a plus.

Unfortunately, I just had to take my main mac for service so I have to get
my much older (still Intel) mac up to speed first/get the original back,
which I probably can't do for a few days.

Maybe @TomAugspurger can take a look?

What's strange is that ultimately `align` will call `join` (with `how='outer'` by default), so I don't see why there should be any difference. I guess the devil is in the details.