[Numpy-discussion] Pull Requests I'm planning to merge

Travis Oliphant travis@continuum...
Tue Jul 17 21:20:25 CDT 2012


On Jul 17, 2012, at 6:48 PM, Nathaniel Smith wrote:

> On Tue, Jul 17, 2012 at 9:56 PM, Travis Oliphant <travis@continuum.io> wrote:
>> 
>> I would like to merge the following pull requests sometime today:
>> 
>> * 326 -- inplace increment function
> 
> -1, for the reasons stated in the comment thread -- we shouldn't lock
> ourselves into an ugly API when there's discussion going on about the
> right solution, and the submitter has expressed interest in rewriting
> it in a better way. Also to be clear, I'm not saying I will try to
> block this functionality forever unless it's perfect or anything -- I
> could probably be convinced that putting in something sub-optimal here
> was the best trade-off. Really what I'm -1 on is shoving something in
> without proper discussion just because the release is happening, on
> the theory that we can clean up any mess later. Until your comment a
> few hours ago no-one even looked at this new API except me and the
> submitter...

You are incorrect about that.  I've looked at it several times since it was posted.   I've looked at and studied all the PRs multiple times in fact over the past several months.  I've waited for others to express an opinion.  I don't think it's accurate to assume that people have not seen something because they don't comment.   

I will hold off on this one, but only because you raised your objection to -1 (instead of -0.5).    But, I disagree very strongly with you that it is "sub-optimal" in the context of NumPy.   I actually think it is a very reasonable solution.   He has re-used the MapIter code correctly (which is the only code that *does* fancy indexing).   The author has done a lot of work to support multiple data-types and satisfy an often-requested feature in a very reasonable way.  Your objection seems to be that you would prefer that it were a method on ufuncs.  I don't think this will work without a *major* refactor.  I'm not sure it's even worth it then either. 

I'm glad to hear you will not block this being added in the future. 

> 
>> * 325 -- prefer gfortran on OSX and Linux
>> * 192 -- meshgrid enhancements
>> * 327 -- restore commas and update C-API doc a bit
> 
> Still needs a test.

That would be nice, but the pull request that removed it also needed a test.  This is just restoring behavior that should be there, so it has to go in before the release --- test or no test, unfortunately. 

> 
>> * 352 -- simplifying case for insert and adding tests (#2028)
>> * 350 -- return view for subset of fields.
>> 
>> Also, I would like to do
>> 
>> * 356 -- with Charles's edited fix in the comments (Bug #808)
> 
> Otherwise looks fine to me.
> 
> -n
> _______________________________________________
> NumPy-Discussion mailing list
> NumPy-Discussion@scipy.org
> http://mail.scipy.org/mailman/listinfo/numpy-discussion



More information about the NumPy-Discussion mailing list