Add ColorBlending module for multi-image blending support#161
Add ColorBlending module for multi-image blending support#161
Conversation
… subsetting and alpha preservation
…iterative probing
…om_imageview_id class method
…olor blending objects
…ology and usage patterns
…or coordinate system and number format methods
…lorBlending initialization
… for consistency with naming conventions
…ColorBlending and Layer method docstrings
…ces for consistency with documentation style
…of setActiveFrameById
…ges and update base path to colorBlendingImageMap
…_layer_sequence method
…Blending.set_colormap_set method
…matching file_name in imageNames array
…ices in set_layer_sequence method
izkgao
added
awaiting code review
confluence
left a comment
confluence
left a comment
There was a problem hiding this comment.
I would like the factory classmethods not to be exposed to the user -- they should have wrappers (in the same way that the Image factory classmethods are wrapped by methods on the session object). I also have a question about what happens if the user interacts with an image object that corresponds to a color blending (will anything break?).
| "data/fits/second_file.fits", | ||
| "data/fits/third_file.fits", | ||
| ] | ||
| cb = ColorBlending.from_files(session, files, append=False) |
There was a problem hiding this comment.
This class method should not be exposed to the user directly. The user should access it through a wrapper on the session object, e.g. something like session.open_as_color_blending([path1, path2, path3]).
This is analogous to the open_images function on the session.
| # Warning: This will break the current spatial matching and | ||
| # use the first image as the spatial reference | ||
| # Note: The base layer (index = 0) cannot be deleted or moved. | ||
| cb = ColorBlending.from_images(session, [img0, img1, img2]) |
There was a problem hiding this comment.
Similarly, this should not be exposed. I'm not sure if it would be best to add the wrapper method to the session object (e.g. maybe session.color_blending_from([img0, img1, img2])) or to the image object (to be called on the image that's the spatial reference, e.g. img0.color_blending_with([img1, img2])).
| cb = color_blendings[0] | ||
|
|
||
| # Or get a color blending object by its image view index | ||
| cb = ColorBlending.from_imageview_id(session, 3) |
There was a problem hiding this comment.
This should also not be exposed. Is each colour blending also an image that appears in the image list? If yes, then the wrapper for this should be on the image object, and be decorated as an attribute, since the only parameter is the image's ID. Then the user could access it with img.color_blending.
Broader question: if each color blending is also an image and can be accessed through an image object in the wrapper, does that break anything (because it's not a normal image)? E.g. what will happen if the user accesses a blending as an image object and tries to get the number of channels?
If this does break image functionality, I think that the correct thing to do is to specialise Image into subclasses (ImageBase, Image, and ColorBlending), but I don't want to overthink this if it's not actually an issue.
| # Get layer objects | ||
| layers = cb.layer_list() | ||
|
|
||
| # Set colormap for individual layers | ||
| layers[0].set_colormap(Colormap.REDS) | ||
| layers[1].set_colormap(Colormap.GREENS) | ||
| layers[2].set_colormap(Colormap.BLUES) |
There was a problem hiding this comment.
I suggest unpacking in the assignment, like this:
# Get layer objects
red, green, blue = cb.layer_list()
# Set colormap for individual layers
red.set_colormap(Colormap.REDS)
green.set_colormap(Colormap.GREENS)
blue.set_colormap(Colormap.BLUES)
| ) | ||
|
|
||
| @classmethod | ||
| def from_imageview_id(cls, session, imageview_id): |
There was a problem hiding this comment.
This is the same as the image object ID, right? For consistency, we should call this image_id.
There was a problem hiding this comment.
imageview_id refers to the index in the image list (see the red box),
while image_id in carta-python refers to fileId in carta-frontend, which is not exposed to users and does not change after moving from index 0 to index 2 in the image list.
There was a problem hiding this comment.
Right, I've remembered how this works now. :) As discussed on slack, I think that we should:
- rename the current
image_idin image objects tofile_id - use
image_idto refer to the image list indices (exposing both in the repr) - use
blending_idandimage_idin the color blending objects - unify Image and ColorBlending into the same object hierarchy using a shared base class
- retrieve both normal files and color blendings in the image list function in the session
- let functions that refer to an image by ID should accept multiple ID types (as appropriate)
- not cache the image ID in the objects (because it can change in the frontend), and fetch it dynamically via the stable internal ID instead
| image_type = session.get_value(f"{path}.type") | ||
| if image_type != ImageType.COLOR_BLENDING: | ||
| raise ValueError( | ||
| "imageview_id does not refer to a color blending image." |
There was a problem hiding this comment.
This message should actually include the value of the ID.
| return self.get_value("filename") | ||
|
|
||
| @property | ||
| def imageview_id(self): |
There was a problem hiding this comment.
Similarly, this should be image_id.
izkgao
added
awaiting code changes
3 participants
Description
Closes #160.
This PR implements the
colorblendingmodule that adds support for creating and manipulating color blended images in CARTA.Checklist