New MaskSet.get_cmap() implementation

[forman]

Closes #1011

@konstntokas please also test with original LC data in last webinar notebook.

Checklist:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/source/*
• Changes documented in CHANGES.md
• GitHub CI passes
• AppVeyor CI passes
• Test coverage remains or increases (target 100%)

[konstntokas]

We still have the same problem that the user will need to add vmin and vmax manually. I find this, as a user, more confusing then getting vmin and vmax from the get_cmap method. See examples below:

land_cover_mask = MaskSet(land_cover_2019.lccs_class)
cmap = land_cover_mask.get_cmap()
land_cover_2019.lccs_class.plot(cmap=cmap)

land_cover_mask = MaskSet(land_cover_2019.lccs_class)
cmap = land_cover_mask.get_cmap()
land_cover_2019.lccs_class.plot(cmap=cmap, vmin=0, vmax=220)
plt.show()

The colormap is not really nice, but this method at least buts the values on the correct position. I did something like:

def get_cmap(maskset):
    vals = maskset._flag_values
    extent = vals.max() - vals.min()
    vals_norm = (vals - vals.min()) / extent
    colors = [(0, 0, 0, 0)] + maskset._flag_colors
    color_mapping = [(v, c) for v, c in zip(vals_norm, colors)]
    for v, c in zip(vals, colors):
        print(v, c)
    return matplotlib.colors.LinearSegmentedColormap.from_list(
        str(maskset._flag_var.name), color_mapping, N=len(vals)
    )

land_cover_mask = MaskSet(land_cover_2019.lccs_class)
cmap = get_cmap(land_cover_mask)
land_cover_2019.lccs_class.plot(cmap=cmap, vmin=0, vmax=220)
plt.show()

This gives an aesthetically nicer colormap but distorts the actual values of course. I am indecisive whats better.

Still we should do something about vmin and vmax, I think. Otherwise a lot of users will run into problems. Would it be possible to add a plotting routine to MaskSet?

[konstntokas]

Why Optional?

[forman]

Forgot to remove.

[forman]

We still have the same problem that the user will need to add vmin and vmax manually.

Then, xarray is not calling the color map in the right way. The new color map is a matplotlib.colors.ListedColormap and it's documentation (and ensured by our unit tests) says:

This may be most useful when indexing directly into a colormap

I find this, as a user, more confusing then getting vmin and vmax from the get_cmap method.

That's right!

[forman]

The colormap is not really nice, but this method at least buts the values on the correct position.

This gives an aesthetically nicer colormap but distorts the actual values of course. I am indecisive whats better.

The Land Cover color mapping is not about aesthetics. The ultimate goal is to have an exact match between flag values and assigned color. It is really a pity that the provided ListedColormap is not used correctly by xarray. I'll need to update the implementation and also return a (cmap, norm) pair then, similar to your code.

[forman]

Still we should do something about vmin and vmax, I think.

Yes, return a (cmap, norm) pair, see above.

Would it be possible to add a plotting routine to MaskSet?

Yes, but let's keep it simple for time being. I will change impl. to return the pair.

[konstntokas]

Including matplotlib.colors.BoundaryNorm the colormap looks like this. The numbers are of course confusing. We would need to create the plot and assign the ticks to the center of each discrete color and assign the tick label to either the flag value or even flag name.

Code snippet:

land_cover_mask = MaskSet(land_cover_2019.lccs_class)
cmap, norm = land_cover_mask.get_cmap()
land_cover_2019.lccs_class.plot.imshow(cmap=cmap, norm=norm)
plt.show()

[forman]

See suggestions

[forman]

Suggested change

	An suitable instance of ```matplotlib.colors.Colormap``` and
	the corresponding ```matplotlib.colors.BoundaryNorm``` if applicable
	An suitable instance of ``matplotlib.colors.Colormap`` and
	the corresponding ``matplotlib.colors.BoundaryNorm`` if applicable

[forman]

Why 0.5 shift?

[konstntokas]

shift is removed, see plot below.

[konstntokas]

The plot now looks like this without the shift of 0.5 as suggested by @forman:

Code snippet

land_cover_mask = MaskSet(land_cover_2019.lccs_class)
cmap, norm = land_cover_mask.get_cmap()
land_cover_2019.lccs_class.plot.imshow(cmap=cmap, norm=norm)
plt.show()

[forman]

I approve & merge, broken CI has other reasons.