[Numpy-discussion] Requesting Code Review of nanmedian ENH
Hi everyone, I put together a np.nanmedian function to extend np.median to handle nans. Could someone review this code and give me some feedback on it before I submit a pull request for it? https://github.com/dfreese/numpy/compare/master...feature;nanmedian Thanks, David ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Requesting Code Review of nanmedian ENH
hi david, I havnt run the code; but the _replace_nan(0) call worries me; especially considering that the unit tests seem to deal with positive numbers exclusively. Have you tested with mixed positive/negative inputs? On Sun, Feb 16, 2014 at 6:13 PM, David Freese dfre...@stanford.edu wrote: Hi everyone, I put together a np.nanmedian function to extend np.median to handle nans. Could someone review this code and give me some feedback on it before I submit a pull request for it? https://github.com/dfreese/numpy/compare/master...feature;nanmedian Thanks, David ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Requesting Code Review of nanmedian ENH
On Sun, Feb 16, 2014 at 12:13 PM, David Freese dfre...@stanford.edu wrote: Hi everyone, I put together a np.nanmedian function to extend np.median to handle nans. Could someone review this code and give me some feedback on it before I submit a pull request for it? It looks good to submit as a pull request but probably will need some changes like the mixed sign thing already mentioned, and I see mean vs. median copypaste remnants in the docstring. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Requesting Code Review of nanmedian ENH
the 0s put into the array copy arr are not used in computation. The _replace_nan call is used primarily to generate a mask of the NaNs and make sure it passes the mutation test. I updated the unit tests to reflect negative values, which works. (and the documentation should be cleaned up now) https://github.com/dfreese/numpy/compare/master...feature;nanmedian On Sun, Feb 16, 2014 at 9:52 AM, alex argri...@ncsu.edu wrote: On Sun, Feb 16, 2014 at 12:13 PM, David Freese dfre...@stanford.edu wrote: Hi everyone, I put together a np.nanmedian function to extend np.median to handle nans. Could someone review this code and give me some feedback on it before I submit a pull request for it? It looks good to submit as a pull request but probably will need some changes like the mixed sign thing already mentioned, and I see mean vs. median copypaste remnants in the docstring. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Requesting Code Review of nanmedian ENH
It doesn't deal with numpy.matrix in the same way as numpy.nanmean. never mind about this -- it looks like np.median is currently broken for np.matrix. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Requesting Code Review of nanmedian ENH
On Sun, Feb 16, 2014 at 1:01 PM, David Freese dfre...@stanford.edu wrote: the 0s put into the array copy arr are not used in computation. The _replace_nan call is used primarily to generate a mask of the NaNs and make sure it passes the mutation test. I updated the unit tests to reflect negative values, which works. (and the documentation should be cleaned up now) https://github.com/dfreese/numpy/compare/master...feature;nanmedian It doesn't deal with numpy.matrix in the same way as numpy.nanmean. For example import numpy as np m = np.matrix([[np.nan, np.nan, 1]]) np.isscalar(np.nanmean(m)) True np.isscalar(np.nanmedian(m)) False Some of the nanfunctions.py code has comments regarding carefulness in dealing with subclasses of numpy.ndarray (like numpy.matrix), and some of the nanfunctions tests include tests for this kind of behavior. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
