On Fri, Mar 30, 2012 at 1:22 PM, Charles R Harris <charlesr.har...@gmail.com> wrote: > > > On Fri, Mar 30, 2012 at 5:41 AM, Nathaniel Smith <n...@pobox.com> wrote: >> >> On Thu, Mar 29, 2012 at 6:53 PM, Tim Cera <t...@cerazone.net> wrote: >> > If instead you passed in a function: >> > >> > def padwithzeros(vector, pad_width, iaxis, **kwargs): >> > bvector = np.zeros(pad_width[0]) >> > avector = np.zeros(pad_width[1]) >> > return bvector, avector >> > >> > b = pad(padwithzeros, a, 2) >> > >> > Would that have some goodness? >> >> I like the idea, but this interface feels undercooked to me. What is >> iaxis? (I couldn't figure that out from the docstrings in the pull >> request either.) If padding a matrix, do we need a way to do padding >> of all rows simultaneously? (Certainly that'd be just as easy for >> something like padwithzeros, and would let us avoid a python-level for >> loop.) Why is this function allocating new arrays that will just be >> copied into the big array and then discarded, instead of filling in >> the big array directly? (Again, this is a speed issue.) If it's >> working with single rows at a time, then how will padwithconstant know >> which constant to pull out of kwargs["constant_value"] when it's an >> array? >> >> That's why I thought you might want to submit this as a second pull >> request, rather than letting it hold up the whole thing. >> > > Would you suggest committing the current PR and then adding this interface > later?
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. 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. 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".) -- Nathaniel _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
