-
-
Notifications
You must be signed in to change notification settings - Fork 11k
ENH: adds support for array construction from iterators #5863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Ehhhhh.... my inclination is to fix this by making the array-coercion logic |
the very last example is a nested iterator. EDIT: no it is not! I was wrong! |
Yes, that's what I'm scared of :-) |
8be100f
to
1b1e8c2
Compare
b50d9a1
to
d7df297
Compare
If you have an idea of how to go forward, sure, but it doesn't seem so easy. Also your last example does not count because range is a sequence since it has a known length. But zip ignores this known length. |
go forward w/ what? what is the problem?
that is true, and i was wrong. so the last example is not nested iterators, and this patch does not try to handle an iterator of iterators, but it does handle this kind of nesting: >>> from itertools import imap, ifilter
>>> it = (x for x in xrange(5))
>>> pred = lambda x: x & 1
>>> op = lambda x: x * 3
>>> np.array(imap(op, ifilter(pred, it)))
array([3, 9]) |
Well, the one thing is always, are we sure that nobody's code may be broken by this? The other is here, whether a half fix is good. Then there is the question (I am not sure, it was extended), whether |
d7df297
to
e8c9ef9
Compare
Dear all, I also would appreciate seeing this branch being merged into master. As far as I can think, the expected functionality of As
Important: Further this would also handle all the rather unintuitive behaviours of I myself use numpy a lot for interactive coding, however, especially with python3 in mind, now generators become the central object to pass around and work with. Having to write my own wrapper functions for np.array, np.mean, np.all, ... is such a big mess ... and the solution is so near: behzadnouri already made the pull request. Considering whether this could break existing code, I cannot imagine any application where
would be useful. Can someone else? This of course would be a crucial point, but at least I think the change is still worth it, especially to make numpy more userfriendly for python3-style scientific interactive coding. cheers |
I sorta feel like if you are constantly getting generators over array data, then you are doing something suboptimal :-). What are you doing that you find yourself constantly wanting to call np.mean and np.all on generators? Generators are like for loops, but with even more overhead, so should be avoided in "numpythonic" code. Also, there isn't really any reliable way to detect an arbitrary iterable in python, making it rather fraught to handle in a generic "do what I mean" interface like np.array. So I'm not at all convinced that this is a good idea. But if you want to move it forward, the next step in any case would be to raise it for discussion on the mailing list. (Ideally with convincing arguments against the concerns raised in this thread :-).) |
thanks for the response!, In general it seems given that it is possible to identify
Given this, I think the general argument goes roughly like the following: PROS (effect maybe 10% of numpy user or more):
CONS (effect less than 0.1% numpy user I would guess):
which in total, at least for me at this stage, speaks in favour of merging In which mailinglist I would have to post this? EDIT: P.S.: sorry that the |
The mailing list is numpy-discussion, Google should find the details for |
Also related to #14539 |
Howdy. |
I am going to close the PR since it is so old and the code has changed too much to salvage anything – besides the many open questions/discussion. Frankly, maybe we should rather reject iterators and improve (and in the error advertise) |
on numpy master, iterators are not iterated in array construction:
this is almost never what one would expect and can cause issues as in #5756, #5951, and gh-6555
with this patch: