-
Notifications
You must be signed in to change notification settings - Fork 1
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
Color by attribute #148
Color by attribute #148
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…f default attributes when fetching
Hi @andy-sweet and @aganders3, This PR is a bit bigger and contains the feature to color cells based on provided attributes. In short, 1) extra columns in the Considerations for review:
Thanks! |
It looks like you're creating another ragged array in the top-level of the Zarr bundle as I agree packing it all into |
Hi @aganders3, thanks for having a look! Yes, that is correct, What do you mean with "making |
Ahh, I see. I missed this distinction and thought the attributes were still interleaved. Never mind my suggestion then, I think this achieves the same thing! |
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.
I'm still making my way through this, but left lots of minor comments and one or two more important ones. Will try to take another look tomorrow.
Qualitatively, I noticed a minor performance hit, but not enough of one to block this feature which seems really useful!
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.
I think the zarr storage format and fetching (when needed) is as reasonable as prior choices we've made, so I don't have any feedback related to that or the Python conversion code/tools.
My main comment is about the how you handle the state related to the attribute drop-down box value changing. I previously left some related comments, but I made those a bit more tangible by trying some changes that I think simplify things and also fix some bugs: https://github.com/royerlab/inTRACKtive/compare/color-by-attribute...andy-sweet/refactor-attr-options?expand=1#diff-8e6c274c4241821e809d16fcd06bc109ba41bb7a0583f9ad766ed92bb8230671R8
I don't think you should blindly merge those changes, but I think the approach there is easier to follow and importantly does not make the library code depend on definitions from React components.
* Rough attempt at refactoring attr options * Use regular dropdown * Add some comments * resolved Zarr3 issue, and npm complaint in leftSidebar/TrackControls --------- Co-authored-by: Andy Sweet <[email protected]>
Thanks a lot for the reviews @andy-sweet and @aganders3 🙌 |
I closed #147, and started a new one on this topic:
This PR will implement the option to color the cells based on some attribute. The attribute could be any column added to the
tracks.csv
, or calculated features from the coordinates (think of velocity, direction, etc.)Implemented:
tracks.csv
into Zarr Store.zattrs
trackMaterial
colormaps)Bugs:
Example datasets:
Example of coloring based on z-coordinate: