Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Optimizations for dtype detection #435

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

87 commented Sep 11, 2012

Hi, these are a few small optimizations to PyArray_DTypeFromObjectHelper.

Use PyType checks to follow different code paths. This includes a general
PyNumber_Check for Python scalars and a PyObject_CheckBuffer check
to avoid running unnecessary buffer code. Also prioritize integer and long
checks in _array_find_python_scalar_type before float and complex, because
they have (much) faster type recognition (since Python 2.6).

@87 87 ENH: Optimize PyArray_DTypeFromObjectHelper
Use PyType checks to follow different code paths. Also prioritize integer
and long paths before float and complex, because they have faster type
recognition.
d19bf86
Owner

teoliphant commented Sep 12, 2012

Please make sure that white-space only changes are removed before doing the pull-request. This makes the review process much easier.

Thanks.

Contributor

87 commented Sep 12, 2012

The white-space changes are because the code is indented one tab (4 spaces) compared to the previous version, because of an additional "if" statement. The diff is not able to detect this, it seems, but the code would not be correct otherwise.

Contributor

87 commented Sep 12, 2012

P.S. The build failure is unrelated to these changes..

Owner

njsmith commented Sep 21, 2012

The changes seem uncontroversial to me. Have you checked that we have good test coverage for this code (since this kind of if-else-spaghetti is easy to break in some obscure way), and have you tried quantifying how much the optimization wins us?

Contributor

87 commented Sep 21, 2012

I'll write up some test cases.

Owner

charris commented Nov 25, 2012

How are the test cases coming?

Owner

charris commented May 3, 2013

@87 Hi Hans, this needs a rebase. Any tests for this?

Owner

charris commented Jul 10, 2013

@87 How is this coming along?
@raulcota This seems like it would complement your work.

Member

seberg commented Nov 22, 2013

Except possibly the first minor part, these things have been included in raulcota and anriks work, and since it is just sitting for a long time anyway, closing it.

@seberg seberg closed this Nov 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment