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

Improve support for mode P images #1844

Closed
gerritholl opened this issue Oct 1, 2021 · 7 comments · Fixed by #2301
Closed

Improve support for mode P images #1844

gerritholl opened this issue Oct 1, 2021 · 7 comments · Fixed by #2301

Comments

@gerritholl
Copy link
Collaborator

Feature Request

Is your feature request related to a problem? Please describe.

It is currently difficult or unclear to work with mode P images in Satpy. For example, the following produces an image in mode RGB:

from satpy import Scene
sc = Scene(
    filenames=["/media/nas/x21308/scratch/NWCSAF/NWCSAF-NinjoTIFF-2/S_NWC_CT_MSG4_MSG-N-VISIR_20210930T100000Z.nc"],
    reader=["nwcsaf-geo"])
sc.load(["cloudtype"])
print(sc["cloudtype"].attrs["mode"])

Users may expect or at least want the option to retain an image in mode P. There are some references on GitHub to mode P, for example, see #1432 and #1817. The composite documentation states only L, LA, RGB, and RGBA are supported.

It's unclear if and how Satpy users can produce images of mode P.

Describe the solution you'd like

I would like a clearly implemented and clearly documented way to produce images of mode P where appropriate.

Describe any changes to existing user workflow

I don't know the current workflow, so I don't know if anything would change. Maybe it's just a matter of improving documentation, or maybe it requires to rewrite all of pytroll from scratch and start over. Most likely the solution lies somewhere in-between.

Additional context

It's possible to hack something together that converts an RGB image back to mode P. This is doubleplusungood.

@gerritholl
Copy link
Collaborator Author

It's possible to write P-mode images by using trollimage and the Writer class directly. For example:

from satpy import Scene
from satpy.writers import to_image
from trollimage.colormap import Colormap
from sattools.io import plotdir
sc = Scene(
    filenames=["/media/nas/x21308/scratch/NWCSAF/NWCSAF-NinjoTIFF-2/S_NWC_CT_MSG4_MSG-N-VISIR_20210930T100000Z.nc"],
    reader=["nwcsaf-geo"])
sc.load(["ct", "ct_pal"])
ct_im = to_image(sc["ct"])
cmap = Colormap(*[(i, tuple(sc["ct_pal"][i, :].compute().data/256)) for i in range(256)])
ct_im.palettize(cmap)
ct_im.save(str(plotdir() / "test-palette.tif"), keep_palette=True, cmap=cmap)

@gerritholl gerritholl self-assigned this Oct 18, 2021
@gerritholl gerritholl added this to To do in PCW Spring 2022 Apr 12, 2022
@gerritholl
Copy link
Collaborator Author

A P-mode image could also retain units information; there is no reason why this should be removed for something like a colorised cloud top height. This would require that ColormapCompositor no longer derives from GenericCompositor. Changing the base class for ColormapCompositor might affect existing user workflow in unexpected ways.

@gerritholl
Copy link
Collaborator Author

Or maybe the whole sets of composites such as cloudtype should be arranged as SingleBandCompositors, and applying the colours should be done in a colorization enhancement. Trollimage already supports producing a P-mode images from a colorization enhancement.

@gerritholl
Copy link
Collaborator Author

Summary of a discussion between @mraspaud and myself: I'll work on a PModeCompositor that will add a palette that the palettizer can understand.

@gerritholl
Copy link
Collaborator Author

gerritholl commented Dec 1, 2022

Downside of a dedicated compositor: we would need to duplicate each and every composite that currently uses the PaletteCompositor for a version that produced mode P rather than mode RGB. That's 28 composites in the satpy distribution alone.

From a user point of view, it would be preferable to do sc.load(["cloud_top_height"], image_mode="P").

@gerritholl
Copy link
Collaborator Author

As @mraspaud has pointed out in slack, this parameter fits better in save_dataset. However, implementing it there would require introducing a backward-incompatibility. Currently sc.load(["cloud_top_height"]) results in sc["cloud_top_height"] with shape (bands, y, x) where bands==3. If we want to offer the choice between P and RGB at the save_datasets stage, then sc.load(["cloud_top_height"]) would have to result in a cloud top height dataset that has shape (y, x). The result of save_dataset would still be the same, unless the user passes enhance=False.

Would such a backward incompatibility be acceptable?

@gerritholl
Copy link
Collaborator Author

The whole colormap creation in pytroll could use some refactoring. We have:

  • the staticmethod satpy.composites.ColormapCompositor.build_colormap;
  • the function satpy.writers.create_colormap;
  • the classmethod trollimage.colormap.Colormap.from_cls.

None of them call each other. The docstring for from_cls is to a large part verbatim identical to the one for create_colormap.

PCW Autumn 2021 automation moved this from To do to Done Jan 12, 2023
PCW Spring 2022 automation moved this from To do to Done Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

1 participant