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

Use the default template's background color for swatches #1872

Closed
wants to merge 5 commits into from

Conversation

joelostblom
Copy link
Contributor

This is a suggestion to use the default template's background color for swatches Instead of always using the plotly template. The potential benefit of this change would be that it is easier to tell which colors sequence go well with the currently used template.

It would be neat if this also checked if there was a PX template set, and I asked a related question about how PX and PIO templates synergize in #1869.

Screenshots:

image

image

image

image

image

image

@nicolaskruchten
Copy link
Contributor

Looks good to me if you can fix the formatting problem in CI?

@joelostblom
Copy link
Contributor Author

joelostblom commented Nov 5, 2019

I believe this is the same CI error as on the other PRs and unrelated to the changes I have made. The error is

error: cannot format /home/circleci/project/packages/python/plotly/plotly/basedatatypes.py: cannot use --safe with this file; failed to parse source file.  AST error message: misplaced type annotation (<unknown>, line 256)

I think that line 256 might need to have collections. added in front of OrderedDict although I'm not sure since that is a comment.

@nicolaskruchten
Copy link
Contributor

If you want to have this check the px.defaults.template and use that I'd be fine with it.

@joelostblom
Copy link
Contributor Author

Thanks, added!

I also added the default width, so that the autoadjusting behavior can be overridden if desired.

width 600 samples:
image

image

@nicolaskruchten
Copy link
Contributor

re the synergization of px and pio templates... Really the way to do this would be to rewrite this little swatch function on top of px.bar somehow :)

@nicolaskruchten
Copy link
Contributor

the plot titles are also pretty ugly at the moment... they used to say px.colors.whatever but after to move into plotly.py, things got shuffled around

@joelostblom
Copy link
Contributor Author

joelostblom commented Nov 6, 2019

Rebased and cleaned up the titles
image

@emmanuelle
Copy link
Contributor

Closing since #1886 was merged, thanks @joelostblom !

@emmanuelle emmanuelle closed this Nov 11, 2019
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.

3 participants