TST Add checks for contiguity of loaded numpy arrays #160
Conversation
This file contains 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
If an array has at least 2d, test if the contiguity is preserved. For some sklearn models, there are attributes whose contiguity changes after loading (they are curiously neither C nor F contiguous). Those are all 1d, however (sometimes after squeezing), so contiguity should not be an issue. There is a known gap with 2d (or higher) object arrays, where contiguity is not always preserved. Since those don't seem to occur in sklearn, we don't fix that error for now.
|
Ready for review @skops-dev/maintainers |
adrinjalali
approved these changes
Sep 30, 2022
Comment on lines
+280
to
+282
| if val1.squeeze().ndim > 1: | ||
| assert val1.flags["C_CONTIGUOUS"] is val2.flags["C_CONTIGUOUS"] | ||
| assert val1.flags["F_CONTIGUOUS"] is val2.flags["F_CONTIGUOUS"] |
There was a problem hiding this comment.
and all the tests pass! nice!
adrinjalali
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #149
If an array is at least 2d, test if the contiguity is preserved. For some sklearn models, there are attributes whose contiguity changes after loading (they are curiously neither C nor F contiguous). Those are all 1d, however (sometimes after squeezing), so contiguity should not be an issue.
There is a known gap with 2d (or higher) object arrays, where contiguity is not always preserved. Since those don't seem to occur in sklearn, we don't fix that error for now.