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

Visualization: having a cmap_name in plot_results kwargs is restrictive. #311

Open
JakeColor opened this issue Dec 5, 2022 · 3 comments

Comments

@JakeColor
Copy link

While working on a recent assignment for 8250, I noticed that the cmap_name in the plot_results function kwargs is overly restrictive.

My use case:
Imagine a user wants to use a custom colormap to visualize model runs, with blues corresponding to one set of seeded runs, reds to another, and so on.

This is not possible in the current code, which only allows use of default matplotlib cmaps via their string name.

@dapatil211
Copy link
Collaborator

Yes this is true. The current visualization code is an early version. We can update the function to instead take in a function that takes in an index and return a color. Would that work for your use case?

@JakeColor
Copy link
Author

JakeColor commented Jan 25, 2023

six weeks later, I do not remember the full syntax, but your solution sounds non-standard...

One alternative approach you might consider for all your visualization functions is adding an optional matplotlib Axes object argument, ex. plot_func(...,ax=None). By default, the plotting functions would generate a new Axes object on which to plot data, but they would use the user's input ax if overriden.

This approach would make solving the original problem as simple as modifying the cmap_name arg to a more general cmap argument with expected type an actual Matplotlib colormap (or recognized string name). I could have then plotted my custom colormaps to the same Axes object by passing to the plot_results function multiple times for different sets of run data.

This approach is also standard in other plotting libraries, for instance Seaborn IIRC.

@dapatil211
Copy link
Collaborator

I see, that makes sense, thanks for the suggestion! I'll update this thread when I create the PR for the changes.

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

No branches or pull requests

2 participants