Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
Hi, On Fri, Sep 14, 2012 at 11:25 PM, Han Genuit hangen...@gmail.com wrote: I think there is something wrong with the implementation.. I would expect each incoming array in PyArray_ConcatenateFlattenedArrays to be flattened and the sizes of all of them added into a one-dimensional shape. Now the shape is two-dimensional, which does not make sense to me. Also the requirement that all sizes must be equal between the incoming arrays only makes sense when you want to stack them into a two-dimensional array, which makes it unnecessarily complicated. The difficulty here is to use PyArray_CopyAsFlat without having to transform/copy each incoming array to the priority dtype, because they can have different item sizes between them, but other than that it should be pretty straightforward, imo. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion Thanks for the feedback. Feeling inadequate to a full understanding of the code there, I've entered an issue for it: https://github.com/numpy/numpy/issues/442 Ondrej - would you consider this a blocker for release? Best, Matthew ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
Okay, sent in a pull request: https://github.com/numpy/numpy/pull/443. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
I was working on the same fix and so I saw your code was similar and merged it. It needs to be back-ported to 1.7.0 Thanks, -Travis On Sep 15, 2012, at 11:06 AM, Han Genuit wrote: Okay, sent in a pull request: https://github.com/numpy/numpy/pull/443. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
Yeah, that merge was fast. :-) Regards, Han ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
It's very nice to get your help.I hope I haven't inappropriately set expectations :-) -Travis On Sep 15, 2012, at 3:14 PM, Han Genuit wrote: Yeah, that merge was fast. :-) Regards, Han ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
You're welcome. I do not have many expectations; only those you can expect from an open source project. ;-) On Sat, Sep 15, 2012 at 10:33 PM, Travis Oliphant tra...@continuum.io wrote: It's very nice to get your help.I hope I haven't inappropriately set expectations :-) -Travis On Sep 15, 2012, at 3:14 PM, Han Genuit wrote: Yeah, that merge was fast. :-) Regards, Han ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
Hi, On Thu, Sep 13, 2012 at 7:00 PM, Matthew Brett matthew.br...@gmail.com wrote: Hi, On Thu, Sep 13, 2012 at 11:31 AM, Matthew Brett matthew.br...@gmail.com wrote: On Wed, Sep 12, 2012 at 4:19 PM, Nathaniel Smith n...@pobox.com wrote: On Wed, Sep 12, 2012 at 2:46 PM, Matthew Brett matthew.br...@gmail.com wrote: Hi, I just noticed that this works for numpy 1.6.1: In [36]: np.concatenate(([2, 3], [1]), 1) Out[36]: array([2, 3, 1]) but the beta release branch: In [3]: np.concatenate(([2, 3], [1]), 1) --- IndexErrorTraceback (most recent call last) /Users/mb312/ipython-input-3-0fa244c8aaa8 in module() 1 np.concatenate(([2, 3], [1]), 1) IndexError: axis 1 out of bounds [0, 1) In the interests of backward compatibility maybe it would be better to raise a warning for this release, rather than an error? Yep, that'd be a good idea. Want to write a patch? :-) https://github.com/numpy/numpy/pull/440 Thinking about the other thread, and the 'number of elements' check, I noticed this: In [51]: np.__version__ Out[51]: '1.6.1' In [52]: r4 = range(4) In [53]: r3 = range(3) In [54]: np.concatenate((r4, r3), None) Out[54]: array([0, 1, 2, 3, 0, 1, 2]) but: In [46]: np.__version__ Out[46]: '1.7.0rc1.dev-ea23de8' In [47]: np.concatenate((r4, r3), None) --- ValueErrorTraceback (most recent call last) /Users/mb312/tmp/ipython-input-47-e354b8880702 in module() 1 np.concatenate((r4, r3), None) ValueError: all the input arrays must have same number of elements The change requiring the same number of elements appears to have been added explicitly by Mark in commit 9194b3af . Mark - what was the reason for that check? Appealing for anyone who might understand that part of the code : there's a check in multiarraymodule.c at around line 477: /* * Figure out the final concatenated shape starting from the first * array's shape. */ for (iarrays = 1; iarrays narrays; ++iarrays) { if (PyArray_SIZE(arrays[iarrays]) != shape[1]) { PyErr_SetString(PyExc_ValueError, all the input arrays must have same number of elements); return NULL; } } I don't understand the following code so I don't know what this check is for... Cheers, Matthew ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
I think there is something wrong with the implementation.. I would expect each incoming array in PyArray_ConcatenateFlattenedArrays to be flattened and the sizes of all of them added into a one-dimensional shape. Now the shape is two-dimensional, which does not make sense to me. Also the requirement that all sizes must be equal between the incoming arrays only makes sense when you want to stack them into a two-dimensional array, which makes it unnecessarily complicated. The difficulty here is to use PyArray_CopyAsFlat without having to transform/copy each incoming array to the priority dtype, because they can have different item sizes between them, but other than that it should be pretty straightforward, imo. ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
On Wed, Sep 12, 2012 at 4:19 PM, Nathaniel Smith n...@pobox.com wrote: On Wed, Sep 12, 2012 at 2:46 PM, Matthew Brett matthew.br...@gmail.com wrote: Hi, I just noticed that this works for numpy 1.6.1: In [36]: np.concatenate(([2, 3], [1]), 1) Out[36]: array([2, 3, 1]) but the beta release branch: In [3]: np.concatenate(([2, 3], [1]), 1) --- IndexErrorTraceback (most recent call last) /Users/mb312/ipython-input-3-0fa244c8aaa8 in module() 1 np.concatenate(([2, 3], [1]), 1) IndexError: axis 1 out of bounds [0, 1) In the interests of backward compatibility maybe it would be better to raise a warning for this release, rather than an error? Yep, that'd be a good idea. Want to write a patch? :-) https://github.com/numpy/numpy/pull/440 Thanks, Matthew ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
Hi, On Thu, Sep 13, 2012 at 11:31 AM, Matthew Brett matthew.br...@gmail.com wrote: On Wed, Sep 12, 2012 at 4:19 PM, Nathaniel Smith n...@pobox.com wrote: On Wed, Sep 12, 2012 at 2:46 PM, Matthew Brett matthew.br...@gmail.com wrote: Hi, I just noticed that this works for numpy 1.6.1: In [36]: np.concatenate(([2, 3], [1]), 1) Out[36]: array([2, 3, 1]) but the beta release branch: In [3]: np.concatenate(([2, 3], [1]), 1) --- IndexErrorTraceback (most recent call last) /Users/mb312/ipython-input-3-0fa244c8aaa8 in module() 1 np.concatenate(([2, 3], [1]), 1) IndexError: axis 1 out of bounds [0, 1) In the interests of backward compatibility maybe it would be better to raise a warning for this release, rather than an error? Yep, that'd be a good idea. Want to write a patch? :-) https://github.com/numpy/numpy/pull/440 Thinking about the other thread, and the 'number of elements' check, I noticed this: In [51]: np.__version__ Out[51]: '1.6.1' In [52]: r4 = range(4) In [53]: r3 = range(3) In [54]: np.concatenate((r4, r3), None) Out[54]: array([0, 1, 2, 3, 0, 1, 2]) but: In [46]: np.__version__ Out[46]: '1.7.0rc1.dev-ea23de8' In [47]: np.concatenate((r4, r3), None) --- ValueErrorTraceback (most recent call last) /Users/mb312/tmp/ipython-input-47-e354b8880702 in module() 1 np.concatenate((r4, r3), None) ValueError: all the input arrays must have same number of elements The change requiring the same number of elements appears to have been added explicitly by Mark in commit 9194b3af . Mark - what was the reason for that check? Best, Matthew ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
Yep, that'd be a good idea. Want to write a patch? :-) https://github.com/numpy/numpy/pull/440 Thinking about the other thread, and the 'number of elements' check, I noticed this: In [51]: np.__version__ Out[51]: '1.6.1' In [52]: r4 = range(4) In [53]: r3 = range(3) In [54]: np.concatenate((r4, r3), None) Out[54]: array([0, 1, 2, 3, 0, 1, 2]) but: In [46]: np.__version__ Out[46]: '1.7.0rc1.dev-ea23de8' In [47]: np.concatenate((r4, r3), None) --- ValueErrorTraceback (most recent call last) /Users/mb312/tmp/ipython-input-47-e354b8880702 in module() 1 np.concatenate((r4, r3), None) ValueError: all the input arrays must have same number of elements The change requiring the same number of elements appears to have been added explicitly by Mark in commit 9194b3af . Mark - what was the reason for that check? This looks like a regression. That should still work. -Travis Best, Matthew ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release
On Wed, Sep 12, 2012 at 2:46 PM, Matthew Brett matthew.br...@gmail.com wrote: Hi, I just noticed that this works for numpy 1.6.1: In [36]: np.concatenate(([2, 3], [1]), 1) Out[36]: array([2, 3, 1]) but the beta release branch: In [3]: np.concatenate(([2, 3], [1]), 1) --- IndexErrorTraceback (most recent call last) /Users/mb312/ipython-input-3-0fa244c8aaa8 in module() 1 np.concatenate(([2, 3], [1]), 1) IndexError: axis 1 out of bounds [0, 1) In the interests of backward compatibility maybe it would be better to raise a warning for this release, rather than an error? Yep, that'd be a good idea. Want to write a patch? :-) -n ___ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion