CARTAvis / carta-python

CARTA scripting wrapper written in Python
GNU General Public License v3.0
0 stars 0 forks source link

Refactor high-level API by grouping functionality under sub-objects #140

Closed confluence closed 3 months ago

confluence commented 1 year ago

I've been thinking about how to organise both the code and the user API to break up the huge image and session classes and group related functionality together, and I think that it would be good to split some items into sub-properties on these objects. This would make the code less unwieldy and also simplify some of the API naming. So far some groups I've identified are: the overlay (session), and the raster, contours, vector overlay, and the about-to-be added regions (image). At a later stage we will probably add other functionality that follows the same pattern. Some functionality (e.g. inspecting image geometry and other properties) I consider to be "basic" functionality which can remain accessible directly from the main object. I am considering adding an images group to the session object, for consistency with the regions group I'm planning to add to the image object (the image-opening functions would be grouped under this object).

This would change the user-facing API, so I think we should do it sooner other than later (and it may also affect the e2e tests). My suggestion would make the API look something like this:

session.overlay.set_coordinate_system(...)
session.images.open(...) # maybe?
img.raster.set_colormap(...)
img.contours.set_colormap(...)
img.vectors.set_colormap(...) # simpler than vector_overlay?
# To be added in upcoming region implementation
img.regions.add_rectangular(...)

Does that make sense?

I was considering transparently aliasing these functions to functions on the main session and image objects (for backwards compatibility), but 1) that would be a lot of boilerplate, and 2) it would complicate the naming. So I think we should commit to the backwards-incompatible change now, before the wrapper has been more widely released.

Originally posted by @confluence in https://github.com/CARTAvis/carta-python/issues/123#issuecomment-1674844674

confluence commented 1 year ago

This was discussed in a comment and on Slack, but didn't have an actual issue.

To do:

confluence commented 6 months ago

This will be resolved by #123.