check_endian code in _get_filled_data is buggy

[keflavich]

the check_endian branch creates a data variable that is never used. It also uses astype which copies the array.

I'm not sure what the intended behavior is here

[astrofrog]

cc @keflavich

[keflavich]

Right, @astrofrog and I were digging into this at one point and tried using view instead of astype, but that led to problems. The data variable is used, though, in the united branch. I'll PR it here.

[keflavich]

OK... I see data being used on master, but I admit some serious git confusion here, as the last time I looked at master it was being treated incorrectly.

[ChrisBeaumont]

I might have clobbered this during a careless rebase -- my bad. In any event, can you add a test that would catch that?

[keflavich]

FYI, there is still a possible bug here: numpy doesn't seem to respect <f4 as a dtype; it immediately converts it to =f4. The tests will fail on a big-endian architecture (if all is correct)

[ChrisBeaumont]

Hrmm I'm getting confused here

What are the situations in which the check_endian kwarg in get_filled_data becomes important? Can we get this issue more directly with two temporary files with the same contents but different endianness, and assert that get_filled_data does the right thing in both cases?

I have to admit I don't fully understand what the warning in the test means, practically speaking

[keflavich]

Your confusion is justified.

If you create (I think even by reading from disk? Not sure here!) a little-endian array on a little endian system, e.g. a mac, numpy interprets it as 'native':

>>> x = np.array([1], dtype='<f4')
>>> x.dtype.byteorder
'='

whereas a big endian array will keep its type

>>> x = np.array([1], dtype='>f4')
>>> x.dtype.byteorder
'>'

Really, what we want to do is check that the type changes when appropriate and check is turned on. To make that test work right, and importantly check that the change is not made when the check is turned off, we need to force numpy to not use =. For all I know this is well documented somewhere but I'm off on other tasks...

[ChrisBeaumont]

In what situations does the endianness matter? You mention non-numpy functions... should we just directly test that those functions operate properly regardless of the underlying endianness?

[keflavich]

This was specifically for the bottleneck package; it requires native-endian arrays.

[ChrisBeaumont]

Ok, how about a test of bottleneck on a non-native endian array? The intent of such a test is more obvious, and should exercise the same functionality, right?

[astrofrog]

Note that bottleneck is not currently installed on Travis, so this functionality is not getting tested properly on Travis. However, you can add it to the pip install line.

[ChrisBeaumont]

Either way, we should probably make such a test optional, and skipped if bottleneck isn't present

[astrofrog]

@keflavich - can you add bottleneck to Travis and also make sure the test gets skipped if bottleneck is not installed? (also we should make sure the test is set up so as to fail if bottleneck is not installed and not skipped).

[astrofrog]

@keflavich - do you think you can realistically do this in the next couple of days? If not, maybe we should re-schedule to 0.2?

[keflavich]

let's skip it for now; it is non-urgent

[keflavich]

@astrofrog I've now added the bottleneck install on travis