Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Nov 22, 2012

apropos #2318

@wesm
Copy link
Member

wesm commented Nov 22, 2012

This should probably get put in a Cython function that uses the NumPy C API before it can ship. The additional overhead (a couple microseconds is significant) is more concerning in the Series constructor than DataFrame

@wesm
Copy link
Member

wesm commented Nov 22, 2012

fwiw The relevant vbenchmark is series_constructor_ndarray

@ghost
Copy link
Author

ghost commented Nov 22, 2012

No problem, I can do that. is it worth it or is this a non-issue?

@wesm
Copy link
Member

wesm commented Nov 22, 2012

It does seem potentially like overkill since the mixed-endianness problem I would expect to be very rare in practice. Hmm

@ghost
Copy link
Author

ghost commented Nov 23, 2012

your call, i'll leave this open for now. let me know either way (or just close).

@juliantaylor
Copy link

how significant of a performance regression is it?
a possible place where this can happen in practice is when dealing with fits files [1].
fits stores its data in big endian format and it is not unusual to just memory map that in with numpy.

[1] http://fits.gsfc.nasa.gov/

@wesm
Copy link
Member

wesm commented Nov 27, 2012

Tabling this til a later time. I did a brief tinker with some big-endian arrays and it's not clear to me where problems might arise-- DataFrame for example casts any input data to little-endian on arrival, though all that conversion might be a bad thing with large data sets. There's a lot of work needing to be done on data type handling in the near future, so it would be good to make a battery of "smoke tests" to ensure responsible handling of big-endian data types.

@ghost
Copy link
Author

ghost commented Feb 8, 2013

This will never get merged. closing. left #2821 as future reminder.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants