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 new colormap registry in matplotlib #413

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

lkluft
Copy link
Member

@lkluft lkluft commented Feb 7, 2023

Also clean-up imports to be less cluttered in the import block and more verbose in-line.

@lkluft lkluft requested a review from olemke February 7, 2023 11:13
@lkluft lkluft marked this pull request as draft February 7, 2023 11:14
@lkluft
Copy link
Member Author

lkluft commented Feb 7, 2023

One might think about if it is the proper behaviour for functions like colors2cmap to register the colormaps, or if they should simply return an object. I guess for the "Typhon colormaps" it is fine to be registered under the ty: prefix, but the others seem to clutter the new registry.

Also clean-up imports to be less cluttered in the import block and more
verbose in-line.
@lkluft lkluft marked this pull request as ready for review February 7, 2023 11:36
@olemke
Copy link
Member

olemke commented Feb 7, 2023

How about adding a keyword to those functions to let the caller decide if the colormap should be added to the registry and set it to False by default to avoid unwanted cluttering of the registry?

@lkluft lkluft merged commit a85b015 into atmtools:master Feb 7, 2023
@lkluft
Copy link
Member Author

lkluft commented Feb 7, 2023

Sounds like a solution 👍 I will play around with both but will -- if at all -- deal with them in a separate PR anyhow

@lkluft lkluft deleted the silence-warnings branch February 7, 2023 15:03
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.

2 participants