[Numpy-discussion] Style for pad implementation in 'pad' namespace or functions under np.lib

Nathaniel Smith njs@pobox....
Fri Mar 30 07:51:21 CDT 2012


On Fri, Mar 30, 2012 at 1:22 PM, Charles R Harris
<charlesr.harris@gmail.com> wrote:
>
>
> On Fri, Mar 30, 2012 at 5:41 AM, Nathaniel Smith <njs@pobox.com> wrote:
>>
>> On Thu, Mar 29, 2012 at 6:53 PM, Tim Cera <tim@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


More information about the NumPy-Discussion mailing list