Skip to content

Basic region support#136

Merged
confluence merged 63 commits intodevfrom
confluence/regions
Apr 9, 2026
Merged

Basic region support#136
confluence merged 63 commits intodevfrom
confluence/regions

Conversation

@confluence
Copy link
Copy Markdown
Collaborator

@confluence confluence commented Aug 16, 2023

This PR implements #80 and adds basic region support to the high-level wrapper. This PR depends on multiple frontend changes made in #2231, so that should be merged first.

Required before this can be moved out of draft:

  • Fill in missing region and annotation functions (lock, focus, style, modify dimensions, etc.)
  • Add support for WCS coordinates and sizes (requires retroactive change to some existing code for consistency)
  • Fill in missing docstrings
  • Add basic tests
  • Factor out BasePathMixin into a separate PR (it should be merged first and used to refactor the existing code to use the same composition pattern as the region functionality)
  • Manually inspect the generated documentation for bugs.

Outstanding issues to be addressed before merging

  • Make sure that the custom code that patches missing frontend features (size, etc.) is consistent with the dev frontend updates.

This PR should be tested against the latest release version of CARTA. There will be a separate PR to remove the custom code in response to the dev frontend fixes, once this is merged and a main branch is created.

@confluence confluence self-assigned this Aug 16, 2023
@confluence confluence marked this pull request as draft August 16, 2023 10:39
…terpreted as pixels. Removed option to set coordinate system in set_center; this must now be done separately. Updated docstrings and tests. Number validator no longer matches numeric strings.
@confluence
Copy link
Copy Markdown
Collaborator Author

@kswang1029 I'm going to go through your comments in more detail, but briefly:

  • My intention was to provide relevant methods to similar objects, but also a consistent interface between different types -- so that, for example, size / set_size do something sensible for any object (even if this option is not available in the GUI). So the lack of 1:1 correspondence with the GUI is intentional -- but perhaps some of my choices are confusing (I agree that having a size for point regions is weird -- but does the line width not change the appearance of the point? I will test this). I will provide a more detailed response.
  • I treated the ruler as a line-like object, and tried to make its interface consistent with the other line objects.
  • It should be easy to add the option to create rectangles and ellipses using corners; I'll look into this.
  • I agree that the property names should be consistent; in some cases I followed the names in the frontend source (which don't always match the UI). I will check these.

…s type entirely (iteraing over the types should only return supported types)
…pling) for rectangle and line. TBD: functions for creating polygons like this from the start; unit tests
…ode center for polylines and polygons. Implemented rotation and translation methods for consistency. Cleaned up unhelpful use of point objects.
…pers to as_poly*, which delete by default. Revise docstrings.
@kswang1029 kswang1029 requested a review from izkgao November 20, 2025 03:40
@izkgao
Copy link
Copy Markdown

izkgao commented Dec 10, 2025

Some comments and questions:

  1. Do we need to unify the inconsistent behaviors on mismatched world / image coordinates for vector, ruler, and line?

add_line only allows (image, image) and (world, world), while add_vector and add_ruler accept (image, world) and (world, image).

# works
vector = regions.add_vector(start=(100, 100), end=('17:56:21.34', '-21:57:23.16'))
# works
ruler = regions.add_ruler(start=(100, 100), end=('17:56:21.34', '-21:57:23.16'))
# does not work
line = regions.add_line(start=(100, 100), end=('17:56:21.34', '-21:57:23.16'))
# ValueError: ('17:56:21.34', '-21:57:23.16') is not a pair of numbers.
  1. set_control_point and set_control_points do not accept world coordinates.
vector.set_control_point(1, ('17:56:21.34', '-21:57:23.16'))
# ValueError: ('17:56:21.34', '-21:57:23.16') is not a pair of numbers.
vector.set_control_points([('17:56:21.70', '-21:57:28.16'), ('17:56:21.34', '-21:57:23.16')])
# ValueError: ('17:56:21.70', '-21:57:28.16') is not a pair of numbers.
  1. Add guards to invalid WCS coordinates

If the returned image coordinates are like (-1.7976931348623157e+308, -1.7976931348623157e+308), an error should be raised before add_region.

# Add a point with invalid WCS coordinates
## image.from_world_coordinate_points([("12:12:12.12", "12:12:12.12")])
## [(-1.7976931348623157e+308, -1.7976931348623157e+308)]
point = regions.add_point(center=("12:12:12.12", "12:12:12.12"))

print(point)
# CartaActionFailed: CARTA scripting action .fetchParameter called with parameters (Macro('frameMap[4].regionSet.regionMap[-1]', 'regionType'),) failed: TypeError: Cannot read properties of undefined (reading 'regionType')

print(regions.list())
# CartaActionFailed: CARTA scripting action .fetchParameter called with parameters (Macro('frameMap[4].regionSet.regionMap[-1]', 'regionType'),) failed: TypeError: Cannot read properties of undefined (reading 'regionType')

# A failed region was created with a negative ID.
print(regions.get_value("regionList"))
# [{'id': 0, 'type': 0}, {'id': -1, 'type': 0}]

# This failed region cannot be cleared.
regions.clear()
print(regions.get_value("regionList"))
# [{'id': 0, 'type': 0}, {'id': -1, 'type': 0}]

@confluence
Copy link
Copy Markdown
Collaborator Author

@izkgao I've gone through your feedback:

  1. This should work now.
  2. This is intentional: control points can be interpreted as positions or sizes, depending on the type of the region, so it would be complicated to validate them generically for any region. However, each region type should already also have a more high-level unambiguously-named property with image and wcs getters and a setter that does accept values in world coordinates. For example, a vector has endpoints (and wcs_endpoints and set_endpoints), and a polygon has vertices (and wcs_vertices, etc.).
  3. What are the bounds for valid WCS coordinates in the frontend? Is this a global limit, or something that varies between images?

@izkgao
Copy link
Copy Markdown

izkgao commented Jan 21, 2026

@confluence

  1. I found that creating regions with pixel coordinates exceeding the range of float32 will fail.
image

@confluence
Copy link
Copy Markdown
Collaborator Author

Per Slack discussion, we should create a separate issue for the bounds problem, and make sure that it's fixed everywhere applicable

@kswang1029
Copy link
Copy Markdown
Contributor

@confluence ready for merging.

@izkgao izkgao added the awaiting merge For pull requests ready for merge or pending backend/protobuf changes label Apr 9, 2026
@confluence confluence merged commit a64b6a2 into dev Apr 9, 2026
6 checks passed
@confluence confluence deleted the confluence/regions branch April 9, 2026 11:55
@confluence confluence removed the awaiting merge For pull requests ready for merge or pending backend/protobuf changes label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support regions/annotations

4 participants