You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2021/09/17 17:17:22 UTC

[GitHub] [incubator-mxnet] barry-jin commented on a change in pull request #20592: Standardize MXNet NumPy Statistical & Linalg Functions

barry-jin commented on a change in pull request #20592:
URL: https://github.com/apache/incubator-mxnet/pull/20592#discussion_r711215778



##########
File path: python/mxnet/numpy/linalg.py
##########
@@ -890,9 +891,13 @@ def eigvalsh(a, UPLO='L'):
     >>> a = np.array([[ 5.4119368 ,  8.996273  , -5.086096  ],
     ...               [ 0.8866155 ,  1.7490431 , -4.6107802 ],
     ...               [-0.08034172,  4.4172044 ,  1.4528792 ]])
-    >>> LA.eigvalsh(a, UPLO='L')
+    >>> LA.eigvalsh(a, upper=False)
     array([-2.87381886,  5.10144682,  6.38623114]) # in ascending order
     """
+    if(upper==False):

Review comment:
       Comparison to `False` in python should be not expression: `if(upper==False)` => `if not upper`

##########
File path: python/mxnet/numpy/multiarray.py
##########
@@ -3591,6 +3589,55 @@ def remainder(x1, x2, out=None, **kwargs):
     """
     return _mx_nd_np.remainder(x1, x2, out=out)
 
+@set_module('mxnet.numpy')
+@wrap_np_binary_func
+def pow(x1, x2):

Review comment:
       We can use assignment to make `pow` the alias of `power` and add notes to inform the user that `pow` is not a official NumPy operator but a standard API. 
   
   Example:
   ```
   pow = power
   pow.__doc__ = ```
       ...
       Notes
       -----
       `pow` is an alias for `power`. It is a standard API in https://data-apis.org/array-api/latest/API_specification/elementwise_functions.html#pow-x1-x2 instead of an official NumPy operator. 
   
       >>> np.pow is np.power
       True
       ...
   ```
   
   We can apply this to other operators in this PR. What do you think. 

##########
File path: python/mxnet/numpy/multiarray.py
##########
@@ -3075,7 +3080,7 @@ def take(a, indices, axis=None, mode='raise', out=None):
 
 
 @set_module('mxnet.numpy')
-def unique(ar, return_index=False, return_inverse=False, return_counts=False, axis=None):
+def unique(ar, return_index=False, return_inverse=False, return_counts=False):

Review comment:
       Reserve axis keyword for backward compatibility. 

##########
File path: python/mxnet/numpy/linalg.py
##########
@@ -890,9 +891,13 @@ def eigvalsh(a, UPLO='L'):
     >>> a = np.array([[ 5.4119368 ,  8.996273  , -5.086096  ],
     ...               [ 0.8866155 ,  1.7490431 , -4.6107802 ],
     ...               [-0.08034172,  4.4172044 ,  1.4528792 ]])
-    >>> LA.eigvalsh(a, UPLO='L')
+    >>> LA.eigvalsh(a, upper=False)
     array([-2.87381886,  5.10144682,  6.38623114]) # in ascending order
     """
+    if(upper==False):
+        UPLO='L'
+    else:
+        UPLO='U'

Review comment:
       Lint check

##########
File path: python/mxnet/numpy/multiarray.py
##########
@@ -54,27 +54,27 @@
 from .utils import _get_np_op
 from .fallback import *  # pylint: disable=wildcard-import,unused-wildcard-import
 from . import fallback
-
+from ..util import wrap_data_api_statical_func

Review comment:
       import `wrap_data_api_statical_func` in line 48

##########
File path: python/mxnet/numpy/multiarray.py
##########
@@ -3591,6 +3589,55 @@ def remainder(x1, x2, out=None, **kwargs):
     """
     return _mx_nd_np.remainder(x1, x2, out=out)
 
+@set_module('mxnet.numpy')
+@wrap_np_binary_func
+def pow(x1, x2):
+    """
+        First array elements raised to powers from second array, element-wise.

Review comment:
       Remove the extra indentation in this docstring. 




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@mxnet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org