On Sat, May 12, 2012 at 1:54 AM, Nathaniel Smith <n...@pobox.com> wrote:
> On Fri, May 11, 2012 at 9:26 PM, T J <tjhn...@gmail.com> wrote: > > On Fri, May 11, 2012 at 1:12 PM, Mark Wiebe <mwwi...@gmail.com> wrote: > >> > >> On Fri, May 11, 2012 at 2:18 PM, Pauli Virtanen <p...@iki.fi> wrote: > >>> > >>> 11.05.2012 17:54, Frédéric Bastien kirjoitti: > >>> > In Theano we use a view, but that is not relevant as it is the > >>> > compiler that tell what is inplace. So this is invisible to the user. > >>> > > >>> > What about a parameter to diagonal() that default to return a view > not > >>> > writable as you said. The user can then choose what it want and this > >>> > don't break the inferface. > >>> [clip] > >>> > >>> Agreed, it seems this is the appropriate way to go on here > >>> `diagonal(copy=True)`. A more obscure alternative would be to add a > >>> separate method that returns a view. > >> > >> > >> This looks like the best way to deal with it, yes. > >> > >> Cheers, > >> Mark > >> > >>> > >>> > >>> I don't think changing the default behavior in a later release is a > good > >>> idea. It's a sort of an API wart, but IMHO better that than subtle code > >>> breakage. > >>> > >>> > > > > copy=True seems fine, but is this the final plan? What about long term, > > should diag() eventually be brought in line with transpose() and > reshape() > > so that it is a view by default? Changing default behavior is certainly > not > > something that should be done all the time, but it *can* be done if > > deprecated appropriately. A more consistent API is better than one with > > warts (if this particular issue is actually seen as a wart). > > This is my question as well. Adding copy=True default argument is > certainly a fine solution for 1.7, but IMHO in the long run it would > be better for diagonal() to return a view by default. If you want to get to a situation where the behavior is changed, adding a temporary new keyword is not a good solution in general. Because this forces you to make the change over 3 different releases: 1. introduce copy=True 2. change to copy=False 3. remove copy kw and step 3 is still breaking existing code, because people started using "copy=False". See the histogram new_behavior keyword as an example of this. (Aside from it > seeming generally more "numpythonic", I see from auditing the code I > have lying around my homedir that it would generally be a free speed > win, and having to remember to type a.diagonal(copy=False) all the > time in order to get full performance seems a bit annoying.) > > I mean, I'm all for conservatism in these things, which is why I > raised the issue in the first place :-). But it also seems like there > should be *some* mechanism for getting there from here (assuming > others agree we want to). There's been grumblings about trying to do > more evolving of numpy in-place instead of putting everything off to > the legendary 2.0, right? > > Unfortunately just putting a deprecation warning on everyone who calls > diagonal() without an explicit copy= argument seems like it would be > *really* obnoxious, though. If necessary we could get more creative... > add a special-purpose ndarray flag like WARN_ON_WRITE so that code > which writes to the returned array continues to work like now, but > also triggers a deprecation warning? I dunno. Something like this could be a solution. Otherwise, just living with the copy is imho much better than introducing a copy kw. Ralf
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
