-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix coords assignment in CombineAdjustFeatures.run #1965
Conversation
@sgratiy FYI, there was a repo configuration setting that was not running the CI for external first-time contributors. I've been told that that issue is now fixed so if you do a force push or something like that it should now properly run the CI. |
ef6cc21
to
c2cef14
Compare
Hey @berl, @neuromusic could you please review this bugfix. |
Use explicit parameter calls when building table coordinates Fix shape definition for the IntensityTable docs
c2cef14
to
ddb6e39
Compare
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.
looks good.
@sgratiy sorry this needs more careful checking of downstream impacts. Can you verify that switching the dimensions in the |
@berl I have added the notebook demonstrating the problem in the main description. |
thanks @sgratiy. |
This PR fixes a bug in with the
CombineAdjustFeatures.run
that was flipping round and channel coordinates in the resultingDecodedIntensityTable
compare to the input intensities.The fix:
Use explicit parameter calls when building table coordinates
Correct shape definition for the IntensityTable docstring
This notebook shows demonstrates the intensity table coordinates before and after the fix.