-
Notifications
You must be signed in to change notification settings - Fork 31
Updates for mixed data algebra based on __array_priority__. #1373
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
d95c7bb
updated files for release 3.9.0 [ci skip]
evgueni-ovtchinnikov 54505e7
Merge branch 'master' of https://github.com/SyneRBI/SIRF
evgueni-ovtchinnikov dedf83c
Merged branch 'master' of https://github.com/SyneRBI/SIRF
evgueni-ovtchinnikov a164114
added numpy array to possible SIRF image data algebra operands
evgueni-ovtchinnikov c53d80b
added to User Guide a subsection on SIRF/numpy data algebra peculiari…
evgueni-ovtchinnikov 9f2ecf8
updated CHANGES.md [ci skip]
evgueni-ovtchinnikov 80b3c07
added tests for #1358
evgueni-ovtchinnikov 57d9e15
attended to Codacy issues
evgueni-ovtchinnikov 0477cc0
simpliied mixed data algebra
evgueni-ovtchinnikov 17867e4
Merge branch 'master' of https://github.com/SyneRBI/SIRF
evgueni-ovtchinnikov f249547
resolved conflicts
evgueni-ovtchinnikov 5a1e82f
Merge branch 'master' of https://github.com/SyneRBI/SIRF
evgueni-ovtchinnikov 6bfdf52
fixed mixing SIRF and numpy data algebra issue #1357
evgueni-ovtchinnikov 9b67e9e
added algebra tests for ax/x, 2/x etc.
evgueni-ovtchinnikov a00488d
Merge branch 'master' of https://github.com/SyneRBI/SIRF
evgueni-ovtchinnikov c3aa576
Merge branch 'master' into mda-priority-fix
evgueni-ovtchinnikov 856c7ee
added the result type checks for mixed data algebra
evgueni-ovtchinnikov 0d8f93f
[ci skip] updated User Guide description of data algebra and CHANGES.md
evgueni-ovtchinnikov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we sure we really want to use
as_array()for these tests? Obviously, if replacing it withasarray()we'd have to be very careful for any operatoins that modify the object.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better safe than sorry IMHO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert_validitieswould raiseAssertionErrorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not necessarily for tests. Probably a good idea to do at least some of the tests with
asarray()as well, as the future, it's what is must likely to hit us.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, replacing every
as_arrayuse with that ofasarrayis perfectly safe asasarraydefaults to copy - to get a view, you needasarray(copy=False)(we probably should keepas_arraymethod in our data container classes for backward compatibility). However, this needs a separate PR I believe, this one just fixes #1357.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no it doesn't. it defaults to view (if the container supports it).
Whatever, I still feel we need to test with the most used function, even if you know in the end it gives the same results (but at some point, it might not)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleting
as_arraywill break compatibility with CIL.