Check dimensions of arrays passed to C++#5246
Closed
mdboom wants to merge 2 commits intomatplotlib:masterfrom
Closed
Check dimensions of arrays passed to C++#5246mdboom wants to merge 2 commits intomatplotlib:masterfrom
mdboom wants to merge 2 commits intomatplotlib:masterfrom
Conversation
This reverts commit 2f5a633. numpy.atleast_3d does not convert an empty array properly.
Check the dimensions on arrays passed to C++ to ensure they are what we expect.
Member
|
Should these also be checking NDIM? |
Member
|
Pull request #5247 tries to catch the particular case of accessing arrays where some dimension is zero. It's not an alternative to this, more a complement, since it only checks one particular kind of error but does it for all users of |
Member
Author
|
@tacaswell: NDIM is already checked in the #5247 is an interesting idea, though the bug it fixes should be less likely with this PR here. I'd like to really examine the performance implications of that before merging. |
This was referenced Oct 19, 2015
Member
Author
|
Closed in favor of #5274. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed fix for #5185.
This checks all arrays that must have particular dimensions to lessen the likelihood of addressing uninitialized memory.
This includes #5241. I waffled about that, but ultimately, I just can't buy that the behavior of Numpy's
atleast_3d, or at least not for our use case here, so better to not use it. We could work around that behavior, but it gets rather messy. The changes here will at least turn the use ofatleast_3dto pass arrays to the C++ side as an exception that we'll catch early, rather than accessing undefined memory.Replaces #5241 and #5245 (but I'm not closing those just yet in case those approaches are ultimately preferred).