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

Fits thumbnails #91

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/api/toasty.image.ImageMode.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ ImageMode

~ImageMode.from_array_info
~ImageMode.make_maskable_buffer
~ImageMode.try_as_pil

.. rubric:: Attributes Documentation

Expand All @@ -38,4 +37,3 @@ ImageMode

.. automethod:: from_array_info
.. automethod:: make_maskable_buffer
.. automethod:: try_as_pil
4 changes: 3 additions & 1 deletion toasty/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,9 @@ def cascade(self, **kwargs):
return self

def make_thumbnail_from_other(self, thumbnail_image):
thumb = thumbnail_image.make_thumbnail_bitmap()
thumb = thumbnail_image.make_thumbnail_bitmap(
self.imgset.pixel_cut_low, self.imgset.pixel_cut_high
)
with self.pio.open_metadata_for_write("thumb.jpg") as f:
thumb.save(f, format="JPEG")
self.imgset.thumbnail_url = "thumb.jpg"
Expand Down
3 changes: 3 additions & 0 deletions toasty/fits_tiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ def tile(
else:
self._tile_tan(cli_progress, parallel, **kwargs)

for img in self.coll.images():
self.builder.make_thumbnail_from_other(img)
break
Copy link

Choose a reason for hiding this comment

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

Stylistically, I like to have blank lines both before and after indenting constructs such as for, if, etc.

self.builder.write_index_rel_wtml(
add_place_for_toast=self.add_place_for_toast,
)
Expand Down
77 changes: 49 additions & 28 deletions toasty/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,6 @@ def make_maskable_buffer(self, buf_height, buf_width):

return Image.from_array(arr)

def try_as_pil(self):
"""
Attempt to convert this mode into a PIL image mode string.

Returns
-------
A PIL image mode string, or None if there is no exact counterpart.
"""
if self == ImageMode.F16x3:
return None
return self.value


def _wcs_to_parity_sign(wcs):
h = wcs.to_header()
Expand Down Expand Up @@ -857,6 +845,7 @@ def asarray(self):
def aspil(self):
"""Obtain the image data as :class:`PIL.Image.Image`.


Returns
-------
If the image was loaded as a PIL image, the underlying object will be
Expand All @@ -868,12 +857,34 @@ def aspil(self):
"""
if self._pil is not None:
return self._pil
if self.mode.try_as_pil() is None:
if self.mode not in (ImageMode.RGB, ImageMode.RGBA):
Copy link

Choose a reason for hiding this comment

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

Hmm, I think that it's probably better to preserve the original semantics here. PIL does support non-RGB(A) image modes, as listed here, and I think the most sensible behavior for this function is for it to succeed if the data array can be expressed as one of those modes.

Also, I forgot to mention this before, but as always, deleting the try_as_pil() function is an API break, which we should try to avoid if there's not a good reason.

raise Exception(
f"Toasty image with mode {self.mode} cannot be converted to PIL"
)

return pil_image.fromarray(self._array)

def coerce_into_pil(self, pixel_cut_low, pixel_cut_high):
"""Coerce the image data into a :class:`PIL.Image.Image` by converting
the data into an ``uint8`` RGB(A) array.

Parameters
----------
pixel_cut_low : number
An value used to stretch the pixel values to the new ``uint8``
range (0 - 255).
pixel_cut_high : number
An value used to stretch the pixel values to the new ``uint8``
range (0 - 255).
"""
array = np.copy(self._array)
array[..., :3] = (array[..., :3] - pixel_cut_low) / (
pixel_cut_high - pixel_cut_low
) * 255 + 0.5
Copy link

Choose a reason for hiding this comment

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

Hmm, now that I think about this part more, I think it will do the wrong thing in some important circumstances.

First. we should always use self.asarray() instead of directly accessing self._array.

But second, if self._pil is not None, maybe we should return it directly? That won't be helpful if the image has an annoying PIL mode like F, though. So maybe this function should really be called coerce_into_rgb (tying in to my previous comment about the semantics of the aspil() method).

Third, if the data array is two-dimensional — which is going to be the most common usage — the array[..., :3] slice will select only the first three columns of the image. The bulk of the array data won't get scaled correctly. Due to the existence of the F16x3 image mode, I think this function will need to branch its behavior depending on whether
the array's shape looks like (H, W) or (H, W, 3). The only (H, W, 4) mode that we have is RGBA.

But fourth, I think we also need to think about whether this function should return either RGB or RGBA, or choose one. It it can return RGBA, we'd need to initialize the alpha channel appropriately, setting it to zero for NaNs and the like. But WWT thumbnails must be delivered as RGB JPEGs. So for the proximate use case, we need to drop down the alpha channel eventually.

So, synthesizing all of that, I think that this function should actually explicitly be coerce_to_rgb_pil(), with logic to fill in all three channels equally for formats like F32. Non-finite pixels (~np.isfinite(array), which includes NaNs) should be mapped to black. If there is already a _pil representation, it should be returned if it is in RGB mode, or if it is in RGBA mode, it should be downconverted sensibly (there's probably a PIL function do this?). Other PIL modes should be handled with the asarray() codepath.


array = np.uint8(np.clip(array, 0, 255))
return pil_image.fromarray(array)

@property
def mode(self):
return self._mode
Expand Down Expand Up @@ -1167,9 +1178,7 @@ def is_completely_masked(self):
elif self.mode == ImageMode.RGBA:
return np.all(i[..., 3] == 0)
else:
raise Exception(
f"unhandled mode `{self.mode}` in is_completely_masked"
)
raise Exception(f"unhandled mode `{self.mode}` in is_completely_masked")

def save(
self, path_or_stream, format=None, mode=None, min_value=None, max_value=None
Expand Down Expand Up @@ -1205,7 +1214,7 @@ def save(
if format in PIL_RGB_FORMATS and mode is None:
mode = ImageMode.RGB
if mode is not None:
pil_image = pil_image.convert(mode.try_as_pil())
pil_image = pil_image.convert(mode.value)
Copy link

Choose a reason for hiding this comment

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

I'm a little wary of dropping the check here since there are ImageMode values that are not valid inputs to Image.convert(). We need to check for those at some point.

pil_image.save(path_or_stream, format=PIL_FORMATS[format])
elif format == "npy":
np.save(path_or_stream, self.asarray())
Expand Down Expand Up @@ -1239,26 +1248,29 @@ def save(
overwrite=True,
)

def make_thumbnail_bitmap(self):
def make_thumbnail_bitmap(self, pixel_cut_low=None, pixel_cut_high=None):
"""Create a thumbnail bitmap from the image.

Parameters
----------
pixel_cut_low : number or ``None`` (the default)
An optional value used to stretch the pixel values to the new range
as defined by pixel_cut_low and pixel_cut_high. Only used if the
image was not loaded as a PIL image, and must be used together with
pixel_cut_high.
pixel_cut_high : number or ``None`` (the default)
An optional value used to stretch the pixel values to the new range
as defined by pixel_cut_low and pixel_cut_high. Only used if the
image was not loaded as a PIL image, and must be used together with
pixel_cut_low.

Returns
-------
An RGB :class:`PIL.Image.Image` representing a thumbnail of the input
image. WWT thumbnails are 96 pixels wide and 45 pixels tall and should
be saved in JPEG format.

"""
if self.mode in (
ImageMode.U8,
ImageMode.I16,
ImageMode.I32,
ImageMode.F32,
ImageMode.F64,
ImageMode.F16x3,
):
raise Exception("cannot thumbnail-ify non-RGB Image")

THUMB_SHAPE = (96, 45)
THUMB_ASPECT = THUMB_SHAPE[0] / THUMB_SHAPE[1]

Expand All @@ -1281,6 +1293,15 @@ def make_thumbnail_bitmap(self):
try:
pil_image.MAX_IMAGE_PIXELS = None
thumb = self.aspil().crop(crop_box)
except:
if pixel_cut_low is None or pixel_cut_high is None:
raise Exception(
(
"Need both pixel_cut_low and pixel_cut_high parameters"
"to be able to coerce Toasty image into PIL format."
)
)
thumb = self.coerce_into_pil(pixel_cut_low, pixel_cut_high).crop(crop_box)
finally:
pil_image.MAX_IMAGE_PIXELS = old_max

Expand Down