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

Harmonize cmap handling in map functions #87

Merged
merged 6 commits into from
May 31, 2023
Merged

Harmonize cmap handling in map functions #87

merged 6 commits into from
May 31, 2023

Conversation

Beauprel
Copy link
Contributor

This PR introduces changes to harmonize colormap handling in gridmap, scattermap and gdfmap.

The goals were to:

  1. Make sure all combinations of the cmap, levels and divergent arguments worked properly in the three functions.
  2. Remove the dependency to the IPCC discrete colormap RGB files.

The following changes were made:

  • fixed bug causing util custom_cmap_norm to not work when the cmap was supplied as a string.

  • vmin, vmax now work in plot_kw of gdfmap

  • fixed bug that happened when levels was a list and divergent was an int in gdfmap

  • removed dependency on discrete IPCC colormaps from gdfmap

  • Removed dependency on discrete IPCC colormaps from gridmap

    • When the colormap is not discrete, custom_cmap_norm works and we just have to feed norm into xr.plot.pcolormesh() like in the other functions
    • However, when the cmap is discrete (levels), because of a bug in xarray (Colormap Normalisation Giving Unexpected/Incorrect Output pydata/xarray#4061 ), a list of levels has to be passed in the levels argument of xarray.plot.pcolormesh( ) . To do so, custom_cmap_norm now returns a list of levels instead of a normalization instance, in certain cases (when levels is not False in gridmap only).
    • Removed the util cbar_ticks, which was useless anyways because xarray already reduces the number of ticks on the cbar when there are too many,
    • Removed the levels argument from create_cmap, and the discrete cmap IPCC data
    • Removed the maximum number of levels in gridmap, which was entirely due to the IPCC files.

Copy link
Contributor

@juliettelavoie juliettelavoie left a comment

Choose a reason for hiding this comment

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

Il restera un peu de travail à faire sur les colorbars comme discuter sur teams, mais je pense que c'est bon pour l'instant !

@@ -457,13 +456,6 @@ def gridmap(
matplotlib.axes.Axes
"""

# checks
if levels:
Copy link
Contributor

Choose a reason for hiding this comment

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

change to docstring accordingly ( for all functions)

@Beauprel
Copy link
Contributor Author

Maybe a possibility to quickly make levels nicer: change the dtype of the linspace calls in custom_cmap_norm, when the data range is big enough.

e.g. np.linspace(1,10, 10, dtype=int)

This could result in uneven intervals, but you can use plt.colorbar(spacing='uniform') to make them all appear the same on the colorbar.

# Conflicts:
#	docs/notebooks/spirograph_docs.ipynb
@sarahclaude sarahclaude merged commit bdfe147 into main May 31, 2023
@sarahclaude sarahclaude deleted the fix_cmaps branch May 31, 2023 20:14
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