[ https://issues.apache.org/jira/browse/ARROW-2354?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16414182#comment-16414182 ]
ASF GitHub Bot commented on ARROW-2354: --------------------------------------- cpcloud commented on a change in pull request #1794: ARROW-2354: [C++] Make PyDecimal_Check() faster URL: https://github.com/apache/arrow/pull/1794#discussion_r177171726 ########## File path: cpp/src/arrow/python/helpers.cc ########## @@ -227,12 +227,16 @@ Status UInt64FromPythonInt(PyObject* obj, uint64_t* out) { } bool PyDecimal_Check(PyObject* obj) { - // TODO(phillipc): Is this expensive? - OwnedRef Decimal; - Status status = ImportDecimalType(&Decimal); - DCHECK_OK(status); - const int32_t result = PyObject_IsInstance(obj, Decimal.obj()); - DCHECK_NE(result, -1) << " error during PyObject_IsInstance check"; + static OwnedRef decimal_type; Review comment: I guess it's a question of which is more stable: the C implementation of the decimal module or the way Python names types? I personally don't feel strongly about either one. If I was forced to choose, I probably stick with the first implementation. It seems less brittle, because it doesn't depend on Python's naming convention, but that intuition could be misleading. Maybe that's a very stable part of Python. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > [C++] PyDecimal_Check() is much too slow > ---------------------------------------- > > Key: ARROW-2354 > URL: https://issues.apache.org/jira/browse/ARROW-2354 > Project: Apache Arrow > Issue Type: Bug > Components: C++, Python > Affects Versions: 0.9.0 > Reporter: Antoine Pitrou > Assignee: Antoine Pitrou > Priority: Major > Labels: pull-request-available > > See https://github.com/apache/arrow/issues/1792 -- This message was sent by Atlassian JIRA (v7.6.3#76005)