-
Notifications
You must be signed in to change notification settings - Fork 26
Fix IndexSource filter mapping #1230
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
f06cd44 to
de52202
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1230 +/- ##
========================================
Coverage 90.43% 90.44%
========================================
Files 132 132
Lines 13940 13947 +7
========================================
+ Hits 12607 12614 +7
Misses 1333 1333 ☔ View full report in Codecov by Sentry. |
|
Just kicking off unit tests not ready for review. |
2094a72 to
c3d62d3
Compare
c3d62d3 to
c35818a
Compare
| _unq, _inv = np.unique(_filter_indices, return_inverse=True) | ||
| # Repack unique_filters | ||
| self.filter_indices = _inv | ||
| self.unique_filters = [copy.copy(src.unique_filters[i]) for i in _unq] |
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.
Noting that see don't necessarily need to be copied, but it should be less risky in case they are mutated.
| np.testing.assert_allclose(_ppB.images[:], _pp.images[1::2], atol=1e-5) | ||
| # Confirm A and B are still equivalent | ||
| np.testing.assert_allclose(_srcB.images[:], _srcA.images[:]) | ||
| np.testing.assert_allclose(_ppB.images[:], _ppA.images[:]) |
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.
See #1236 for additional tests that might fit here.
|
The GH error appears to have resolved itself 🎉 . |
|
Just got word my reviewer is still sick, so I'm going to merge this. Besides the tests added in the PR, I've successfully run the halfset recon that spawned this issue. |
Attempting to fix the mapping of unique filters through IndexSource.