On Fri, Mar 30, 2012 at 2:20 PM, Tim Cera <t...@cerazone.net> wrote: >> My suggestion is: >> Step 1: Change the current PR so that it has only one user-exposed >> function, something like pad(..., mode="foo"), and commit that. >> Everyone seems to pretty much like that interface, implementing it >> would take <1 hour of work, and then the basic functionality would be >> landed and done. > > This is all done in my working directory.
Cool. > Currently I have 'mode' as the first argument and not a keyword. Could you > explain the utility of having it be a keyword, if that is indeed what you > were advocating earlier? Eh, just that it feels vaguely more consistent with all the other np functions, which generally take arrays as their first argument and sometimes have a mode="string" argument. Examples of the latter: np.linalg.qr, np.correlate, np.convolve, np.take, np.put, np.choose And unlike Charles, my first guess at the default would have been zero-padding (isn't that the most common sort of padding?). But both of these are minor quibbles -- bikeshedding, really. My opinion isn't strong. >> Step 2: Add the option to pass a user-defined function as the mode= >> argument, since there's still uncertainty about the best way to do it >> and working through uncertainty adds time and risk that shouldn't hold >> up the parts that we do agree on. > > > This is done also. I don't do any checks. If it isn't a string, then I take > it to be a function. Yes, that's probably the Right Way in python. > The function signature is: > myfunc(vector, pad_tuple, iaxis, kwds) > and it has to return a rank 1 array the same length as the input `vector` From your other mail, this API does make more sense to me now. However, looking at the code, I think the returned vector is ignored, and ideally should be [] so as to prevent apply_along_axis from generating a large temporary matrix that we will in any case discard? >> Even if we do want to keep around the pad_with_mean, pad_with_median >> etc. functions as additional user-exposed entry-points, I think the >> current names in the PR met with objections? (The current names are >> like "np.lib.pad.pad_mean".) > > Names? I wasn't sure if Charles was talking about merging the current pull request as-is, so I was re-raising the original question you started the thread with: whether we like having the functions referred to like "np.lib.pad.pad_mean", or something else would be better (e.g. "np.lib.pad_with_mean"). If you're changing the PR anyway then you can just ignore that paragraph :-). Cheers, -- Nathaniel _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
