[GitHub] [incubator-mxnet] reminisce commented on a change in pull request #15418: [numpy] fix cython
reminisce commented on a change in pull request #15418: [numpy] fix cython URL: https://github.com/apache/incubator-mxnet/pull/15418#discussion_r299237059 ## File path: python/mxnet/cython/ndarray.pyx ## @@ -64,21 +64,29 @@ cdef class NDArrayBase: _ndarray_cls = None +_np_ndarray_cls = None def _set_ndarray_class(cls): global _ndarray_cls _ndarray_cls = cls -cdef NewArray(NDArrayHandle handle, int stype=-1): +def _set_np_ndarray_class(cls): +global _np_ndarray_cls +_np_ndarray_cls = cls + + +cdef NewArray(NDArrayHandle handle, int is_np_op, int stype=-1): Review comment: 1. Rename `is_np_op` to `is_np_array` for better readability. 2. Better not to change the original API. You can switch `is_np_array` with `stype` and default `is_np_array` to 0. This is an automated message from the Apache Git Service. To respond to the message, please log on to 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] reminisce commented on a change in pull request #15418: [numpy] fix cython
reminisce commented on a change in pull request #15418: [numpy] fix cython URL: https://github.com/apache/incubator-mxnet/pull/15418#discussion_r299237553 ## File path: python/mxnet/cython/ndarray.pyx ## @@ -64,21 +64,29 @@ cdef class NDArrayBase: _ndarray_cls = None +_np_ndarray_cls = None def _set_ndarray_class(cls): global _ndarray_cls _ndarray_cls = cls -cdef NewArray(NDArrayHandle handle, int stype=-1): +def _set_np_ndarray_class(cls): +global _np_ndarray_cls +_np_ndarray_cls = cls + + +cdef NewArray(NDArrayHandle handle, int is_np_op, int stype=-1): """Create a new array given handle""" -return _ndarray_cls(_ctypes.cast(handle, _ctypes.c_void_p), stype=stype) +if is_np_op: Review comment: These four lines can be simplified into two: ```python create_array_fn = _np_ndarray_cls if is_np_array else _ndarray_cls return create_array_fn(_ctypes.cast(handle, _ctypes.c_void_p), stype=stype) ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to 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 With regards, Apache Git Services
[GitHub] [incubator-mxnet] reminisce commented on a change in pull request #15418: [numpy] fix cython
reminisce commented on a change in pull request #15418: [numpy] fix cython URL: https://github.com/apache/incubator-mxnet/pull/15418#discussion_r299238803 ## File path: python/mxnet/cython/symbol.pyx ## @@ -84,19 +84,29 @@ cdef SymbolSetAttr(SymbolHandle handle, dict kwargs): _symbol_cls = SymbolBase +_np_symbol_cls = None def _set_symbol_class(cls): global _symbol_cls _symbol_cls = cls -cdef NewSymbol(SymbolHandle handle): + +def _set_np_symbol_class(cls): +global _np_symbol_cls +_np_symbol_cls = cls + + +cdef NewSymbol(SymbolHandle handle, int is_np_op): Review comment: `is_np_op` --> `is_np_sym` This is an automated message from the Apache Git Service. To respond to the message, please log on to 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 With regards, Apache Git Services
