Skip to content
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

Modified fluorophore color code #202

Merged
merged 4 commits into from
Nov 13, 2023
Merged

Modified fluorophore color code #202

merged 4 commits into from
Nov 13, 2023

Conversation

ieivanov
Copy link
Contributor

@ieivanov ieivanov commented Nov 7, 2023

Suggesting a modification of the display color for some fluorophores.

I found that Cy5 and mCherry were both plotted in magenta, when they actually have quite different emission spectrum.

Alexa561 was also listed under both magenta and yellow colors.

@ziw-liu ziw-liu added the NGFF OME-NGFF (OME-Zarr format) label Nov 7, 2023
Copy link
Contributor

@edyoshikun edyoshikun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise these suggestions LGTM! I also noticed that on the new Opencell dataset. Thanks @ieivanov

"blue": ["DAPI", "Blue", "BFP"],
"red": ["Red"],
"orange": ["Orange", "Cy3"],
"yellow": ["Alexa561"],
"yellow": ["Cy3"], # Cy3 emission is at 570 nm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are at it, should we add the color names as values of the color keys? i.e can we add yellow as an option for yellow and for whatever color?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alexa561 also needs to be yellow then.

@ieivanov
Copy link
Contributor Author

ieivanov commented Nov 8, 2023

If you ask me, I'd keep these default fluorophore colors to the bare minimum. If we start adding esoteric fluorophores, then there would be no end to this. I'd suggest removing GCaMP, mNeon, dTomato, Alexa561, Alexa568, MitoViewGreen, MitoView633, LysotrackerRed, etc. Also, isn't white the default color? In that case, I'd remove that whole key

@ziw-liu
Copy link
Collaborator

ziw-liu commented Nov 9, 2023

Also, isn't white the default color? In that case, I'd remove that whole key

Good point. We inherited the 'white' part from the legacy waveorder writer.

@ieivanov
Copy link
Contributor Author

OK, I've cut down the number of supported fluor to what I think is the bare minimum. Let me know what you think

@edyoshikun
Copy link
Contributor

LGTM!

@ieivanov ieivanov merged commit 64090bc into main Nov 13, 2023
@ziw-liu ziw-liu deleted the new_colors branch November 13, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NGFF OME-NGFF (OME-Zarr format)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants