-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG? hstack/vstack/column_stack no longer accept generators #12263
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
Comments
This is definitely due to my This is certainly not supported behavior -- the docstrings for these functions describes these parameters as accepting a "sequence of ndarrays" and generators are not sequences. That said, if scipy relied on this behavior it would be better to handle this change more gracefully. My proposal:
This will result in a small performance penalty, but I don't see any good alternatives. In particular, I don't see any way we could correctly implement these functions with dispatching with support for generator arguments. |
Scikit-learn relied on this also in many places. Even if it wasn't officially supported I would think a deprecation cycle / FutureWarning would be good. If scipy and scikit-learn rely on it, chances are lots of people were. |
@shoyer That sounds like a good solution to me. |
Yeah, sounds sensible given the strong indication of downstream dependency from big projects. |
I think we will need this fix for
|
To be honest I don't see the benefit of generators in this context since at some point all elements have to be ... generated and be present in the memory all at once. Thus I would be in favor of deprecating and eventually removing them if they make |
For scikit-learn, I think they used this just because it looked a little cleaner with two fewer characters, e.g., self.rows_ = np.vstack(self.row_labels_ == c
for c in range(self.n_clusters)) vs self.rows_ = np.vstack([self.row_labels_ == c
for c in range(self.n_clusters)]) |
@shoyer that seems a likely explanation and I don't think we actually care about adding two characters. Obviously we do care about scikit-learn breaking with the next numpy release, but I think we're all on the same page there, i.e. do a FutureWarning and eventually remove. |
Fixes numpygh-12263 We can't support generators with dispatch for ``__array_function__``.
This is also the cause of test failures for pandas: pandas-dev/pandas#23172 |
As reported by @ilayn over in #12262:
The text was updated successfully, but these errors were encountered: