-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: Fix bug with size 1-dims in CreateSortedStridePerm #2696
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
This changes CreateSortedStridedPerm to not use the shape for special casing 1-dim axis. The cleanup does not seem to be useful in most cases and the current way is buggy. Also insert stride so that reduce with keepdims=1 will keep contiguous arrays contiguous. "Closes Issue numpy#434"
I don't know if this needs to up the C-ABI/API versions or not. |
Looks fine to me. @certik: The background here is that #2694 is going to change the call signature for |
@seberg you don't need to update the API/ABI versions. That should be done by the release manager just before a release. For master or unreleased branches you can get a CAPIMismatchWarning during building, but that's not a big deal. |
Ok. Where is the API/ABI version? I am only aware of these lines in
I'll wait a little more with the merge, to give others chance to comment. If there are no more objections, I'll merge it tomorrow. |
First of, what we are talking about here is ie. a 3x1x3 Fortran ordered array, that is put into I am tending toward just doing it like this PR does and then maybe implementing that extra function later. But if you prefer that |
That's fine with me. Since nobody is against, I am merging it now. |
BUG: Fix bug with size 1-dims in CreateSortedStridePerm
This changes CreateSortedStridedPerm to not use the shape for special casing 1-dim axis. The cleanup does not seem to be useful in most cases and the current way is buggy. Also insert stride so that reduce with keepdims=1 will keep contiguous arrays contiguous. "Closes Issue #434"
Sorry if a PR against 1.7. is the wrong thing at this point. But to me this seems the easiest way to avoid any headaches.