Discussion:
[Numpy-discussion] broadcasting for randint
G Young
2016-05-22 01:15:00 UTC
Permalink
Hi,

I have had a PR <https://github.com/numpy/numpy/pull/6938> open for quite
some time now that allows arguments to broadcast in *randint*. While the
functionality is fully in-place and very robust, the obstacle at this point
is the implementation.

When the *dtype* parameter was added to *randint* (see here
<https://github.com/numpy/numpy/pull/6910>), a big issue with the
implementation was that it created so much duplicate code that it would be
a huge maintenance nightmare. However, this was dismissed in the original
PR message because it was believed that template-ing would be trivial,
which seemed reasonable at the time.

When I added broadcasting, I introduced a template system to the code that
dramatically cut down on the duplication. However, the obstacle has been
whether or not this template system is too *ad hoc* to be merged into the
library. Implementing a template in Cython was not considered sufficient
and is in fact very tricky to do, and unfortunately, I have not received
any constructive suggestions from maintainers about how to proceed, so I'm
opening this up to the mailing to see whether or not there are better
alternatives to what I did, whether this should be merged as it, or whether
this should be tabled until a better template can be found.

Thanks!
G Young
2016-06-07 02:04:00 UTC
Permalink
Just wanted to ping the mailing list again in case this email (see below)
got lost in your inboxes. Would be great to get some feedback on this!
Thanks!
Post by G Young
Hi,
I have had a PR <https://github.com/numpy/numpy/pull/6938> open for quite
some time now that allows arguments to broadcast in *randint*. While the
functionality is fully in-place and very robust, the obstacle at this point
is the implementation.
When the *dtype* parameter was added to *randint* (see here
<https://github.com/numpy/numpy/pull/6910>), a big issue with the
implementation was that it created so much duplicate code that it would be
a huge maintenance nightmare. However, this was dismissed in the original
PR message because it was believed that template-ing would be trivial,
which seemed reasonable at the time.
When I added broadcasting, I introduced a template system to the code that
dramatically cut down on the duplication. However, the obstacle has been
whether or not this template system is too *ad hoc* to be merged into the
library. Implementing a template in Cython was not considered sufficient
and is in fact very tricky to do, and unfortunately, I have not received
any constructive suggestions from maintainers about how to proceed, so I'm
opening this up to the mailing to see whether or not there are better
alternatives to what I did, whether this should be merged as it, or whether
this should be tabled until a better template can be found.
Thanks!
G Young
2016-06-07 17:23:59 UTC
Permalink
There seems to be a push in my PR now for using Tempita as a way to solve
this issue with the ad-hoc templating. However, before I go about
attempting this, it would be great to receive feedback from other
developers on this, especially from some of the numpy maintainers. Thanks!
Post by G Young
Just wanted to ping the mailing list again in case this email (see below)
got lost in your inboxes. Would be great to get some feedback on this!
Thanks!
Post by G Young
Hi,
I have had a PR <https://github.com/numpy/numpy/pull/6938> open for
quite some time now that allows arguments to broadcast in *randint*.
While the functionality is fully in-place and very robust, the obstacle at
this point is the implementation.
When the *dtype* parameter was added to *randint* (see here
<https://github.com/numpy/numpy/pull/6910>), a big issue with the
implementation was that it created so much duplicate code that it would be
a huge maintenance nightmare. However, this was dismissed in the original
PR message because it was believed that template-ing would be trivial,
which seemed reasonable at the time.
When I added broadcasting, I introduced a template system to the code
that dramatically cut down on the duplication. However, the obstacle has
been whether or not this template system is too *ad hoc* to be merged
into the library. Implementing a template in Cython was not considered
sufficient and is in fact very tricky to do, and unfortunately, I have not
received any constructive suggestions from maintainers about how to
proceed, so I'm opening this up to the mailing to see whether or not there
are better alternatives to what I did, whether this should be merged as it,
or whether this should be tabled until a better template can be found.
Thanks!
G Young
2016-06-17 04:02:43 UTC
Permalink
Hello all,

Thank you to those who commented on this PR and for pushing it to a *much
better place* in terms of templating with Tempita. With that issue out of
the way it seems, the momentum has stalled a bit. However, it would be
great to receive any additional feedback, *especially from maintainers* so
as to help get this merged! Thanks!
Post by G Young
There seems to be a push in my PR now for using Tempita as a way to solve
this issue with the ad-hoc templating. However, before I go about
attempting this, it would be great to receive feedback from other
developers on this, especially from some of the numpy maintainers. Thanks!
Post by G Young
Just wanted to ping the mailing list again in case this email (see below)
got lost in your inboxes. Would be great to get some feedback on this!
Thanks!
Post by G Young
Hi,
I have had a PR <https://github.com/numpy/numpy/pull/6938> open for
quite some time now that allows arguments to broadcast in *randint*.
While the functionality is fully in-place and very robust, the obstacle at
this point is the implementation.
When the *dtype* parameter was added to *randint* (see here
<https://github.com/numpy/numpy/pull/6910>), a big issue with the
implementation was that it created so much duplicate code that it would be
a huge maintenance nightmare. However, this was dismissed in the original
PR message because it was believed that template-ing would be trivial,
which seemed reasonable at the time.
When I added broadcasting, I introduced a template system to the code
that dramatically cut down on the duplication. However, the obstacle has
been whether or not this template system is too *ad hoc* to be merged
into the library. Implementing a template in Cython was not considered
sufficient and is in fact very tricky to do, and unfortunately, I have not
received any constructive suggestions from maintainers about how to
proceed, so I'm opening this up to the mailing to see whether or not there
are better alternatives to what I did, whether this should be merged as it,
or whether this should be tabled until a better template can be found.
Thanks!
Loading...