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

Consistent colors for vega photometry plots #1831

Closed
wants to merge 2 commits into from

Conversation

scizen9
Copy link
Contributor

@scizen9 scizen9 commented Mar 31, 2021

Here is an initial try, but it only parses ztf filters. I have to work on a way to parse all the filters.

…rently only parses ztf filters, in VegaPlot.jsx
@scizen9
Copy link
Contributor Author

scizen9 commented Mar 31, 2021

Here's what it does so far:
image

@scizen9
Copy link
Contributor Author

scizen9 commented Mar 31, 2021

Attempting to address Issue #57 and Issue #681.

@acrellin
Copy link
Member

@scizen9 What's the status here?

@dmitryduev
Copy link
Contributor

@scizen9 any updates here?

You may want to use the same logic I used here for Centroid plots.

@acrellin
Copy link
Member

@scizen9 What's the status here? Any way I can help?

@scizen9
Copy link
Contributor Author

scizen9 commented Oct 26, 2021

Vega lite issue still open.

@dmitryduev
Copy link
Contributor

@scizen9 that issue is still open indeed, but it shouldn't be a showstopper here.
I implemented a workaround for the centroid plots, see here https://github.com/fritz-marshal/fritz/blob/master/extensions/skyportal/static/js/components/CentroidPlot.jsx#L108. There's a pre-defined set of key -> color mappings; if a given key is not found, a random color is used instead.

@acrellin
Copy link
Member

@scizen9 Any updates here? According to Dima's comment above, this is currently doable. Thanks!

@acrellin
Copy link
Member

@scizen9 Any updates here?

@profjsb
Copy link
Collaborator

profjsb commented Jan 18, 2022

@scizen9 please followup with Vega developers...

@dmitryduev
Copy link
Contributor

@profjsb no need to do that -- see my comment above (I figured out what needs to be done in an older pr).

@mcoughlin
Copy link
Contributor

Closing in favor of #2734.

@mcoughlin mcoughlin closed this Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants