Re: [Numpy-discussion] Change in behavior of np.concatenate for upcoming release

2012-09-15 Thread Matthew Brett
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

2012-09-15 Thread Han Genuit
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

2012-09-15 Thread Travis Oliphant
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

2012-09-15 Thread Han Genuit
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

2012-09-15 Thread Travis Oliphant
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

2012-09-15 Thread Han Genuit
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

2012-09-14 Thread Matthew Brett
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

2012-09-14 Thread Han Genuit
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

2012-09-13 Thread Matthew Brett
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

2012-09-13 Thread Matthew Brett
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

2012-09-13 Thread Travis Oliphant
 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

2012-09-12 Thread Nathaniel Smith
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