Skip to content

Remove interactive marking API #41

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

Merged
merged 4 commits into from
May 22, 2025
Merged
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
51 changes: 0 additions & 51 deletions src/astro_image_display_api/dummy_viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class ImageViewer:
image_width: int = 0
image_height: int = 0
zoom_level: float = 1
_is_marking: bool = False
stretch_options: tuple = ("linear", "log", "sqrt")
autocut_options: tuple = ("minmax", "zscale", "asinh", "percentile", "histogram")
_cursor: str = ImageViewerInterface.ALLOWED_CURSOR_LOCATIONS[0]
Expand All @@ -46,9 +45,6 @@ class ImageViewer:
# Default marker name for marking via API
DEFAULT_MARKER_NAME: str = ImageViewerInterface.DEFAULT_MARKER_NAME

# Default marker name for interactive marking
DEFAULT_INTERACTIVE_MARKER_NAME: str = ImageViewerInterface.DEFAULT_INTERACTIVE_MARKER_NAME

# some internal variable for keeping track of viewer state
_interactive_marker_name: str = ""
_previous_click_center: bool = False
Expand All @@ -60,18 +56,12 @@ class ImageViewer:
_center: tuple[float, float] = (0.0, 0.0)

# Some properties where we need to control what happens
@property
def is_marking(self) -> bool:
return self._is_marking

@property
def click_center(self) -> bool:
return self._click_center

@click_center.setter
def click_center(self, value: bool) -> None:
if self.is_marking:
raise ValueError("Cannot set click_center while marking is active.")
self._click_center = value
self._click_drag = not value

Expand All @@ -80,8 +70,6 @@ def click_drag(self) -> bool:
return self._click_drag
@click_drag.setter
def click_drag(self, value: bool) -> None:
if self.is_marking:
raise ValueError("Cannot set click_drag while marking is active.")
self._click_drag = value
self._click_center = not value

Expand Down Expand Up @@ -198,45 +186,6 @@ def save(self, filename: str | os.PathLike, overwrite: bool = False) -> None:
p.write_text("This is a dummy file. The viewer does not save anything.")

# Marker-related methods
def start_marking(self, marker_name: str | None = None, marker: Any = None) -> None:
"""
Start interactive marking of points on the image.

Parameters
----------
marker_name : str, optional
The name of the marker set to use. If not given, a unique
name will be generated.
"""
self._is_marking = True
self._previous_click_center = self.click_center
self._previous_click_drag = self.click_drag
self._previous_marker = self.marker
self._previous_scroll_pan = self.scroll_pan
self._click_center = False
self._click_drag = False
self.scroll_pan = True
self._interactive_marker_name = marker_name if marker_name else self.DEFAULT_INTERACTIVE_MARKER_NAME
self.marker = marker if marker else self.DEFAULT_INTERACTIVE_MARKER_NAME

def stop_marking(self, clear_markers: bool = False) -> None:
"""
Stop interactive marking of points on the image.

Parameters
----------
clear_markers : bool, optional
If `True`, clear the markers that were created during
interactive marking. Default is `False`.
"""
self._is_marking = False
self.click_center = self._previous_click_center
self.click_drag = self._previous_click_drag
self.scroll_pan = self._previous_scroll_pan
self.marker = self._previous_marker
if clear_markers:
self.remove_markers(self._interactive_marker_name)

def add_markers(self, table: Table, x_colname: str = 'x', y_colname: str = 'y',
skycoord_colname: str = 'coord', use_skycoord: bool = False,
marker_name: str | None = None) -> None:
Expand Down
36 changes: 0 additions & 36 deletions src/astro_image_display_api/interface_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
ALLOWED_CURSOR_LOCATIONS = ('top', 'bottom', None)

DEFAULT_MARKER_NAME = 'default-marker-name'
DEFAULT_INTERACTIVE_MARKER_NAME = 'interactive-markers'

# List of marker names that are for internal use only
RESERVED_MARKER_SET_NAMES = ('all',)
Expand All @@ -34,7 +33,6 @@ class ImageViewerInterface(Protocol):
image_width: int
image_height: int
zoom_level: float
is_marking: bool
stretch_options: tuple
autocut_options: tuple
cursor: str
Expand All @@ -52,9 +50,6 @@ class ImageViewerInterface(Protocol):
# Default marker name for marking via API
DEFAULT_MARKER_NAME: str = DEFAULT_MARKER_NAME

# Default marker name for interactive marking
DEFAULT_INTERACTIVE_MARKER_NAME: str = DEFAULT_INTERACTIVE_MARKER_NAME

# The methods, grouped loosely by purpose

# Methods for loading data
Expand Down Expand Up @@ -113,37 +108,6 @@ def save(self, filename: str | os.PathLike, overwrite: bool = False) -> None:
"""
raise NotImplementedError

# Marker-related methods
@abstractmethod
def start_marking(self, marker_name: str | None = None, marker: Any = None) -> None:
"""
Start interactive marking of points on the image.

Parameters
----------
marker_name : str, optional
The name of the marker set to use. If not given, a unique
name will be generated.

marker : Any, optional
The marker to use. If not given, a default marker will be
used.
"""
raise NotImplementedError

@abstractmethod
def stop_marking(self, clear_markers: bool = False) -> None:
"""
Stop interactive marking of points on the image.

Parameters
----------
clear_markers : bool, optional
If `True`, clear the markers that were created during
interactive marking. Default is `False`.
"""
raise NotImplementedError

@abstractmethod
def add_markers(self, table: Table, x_colname: str = 'x', y_colname: str = 'y',
skycoord_colname: str = 'coord', use_skycoord: bool = False,
Expand Down
78 changes: 4 additions & 74 deletions src/astro_image_display_api/widget_api_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# TODO: How to enable switching out backend and still run the same tests?
import warnings

import pytest

Expand Down Expand Up @@ -65,7 +64,6 @@ def _get_marker_names_as_set(self):
def test_default_marker_names(self):
# Check only that default names are set to a non-empty string
assert self.image.DEFAULT_MARKER_NAME
assert self.image.DEFAULT_INTERACTIVE_MARKER_NAME

def test_width_height(self):
assert self.image.image_width == 250
Expand Down Expand Up @@ -125,75 +123,15 @@ def test_zoom(self):
self.image.zoom(2)
assert self.image.zoom_level == 6 # 3 x 2

def test_marking_operations(self):
marks = self.image.get_markers(marker_name="all")
self._assert_empty_marker_table(marks)
assert not self.image.is_marking

# Ensure you cannot set it like this.
with pytest.raises(AttributeError):
self.image.is_marking = True

# Setting these to check that start_marking affects them.
self.image.click_center = True # Disables click_drag
assert self.image.click_center
self.image.scroll_pan = False
assert not self.image.scroll_pan

def test_marker_properties(self):
# Set the marker style
marker_style = {'color': 'yellow', 'radius': 10, 'type': 'cross'}
self.image.marker = marker_style
m_str = str(self.image.marker)
for key in marker_style.keys():
assert key in m_str

self.image.start_marking(marker_name='markymark', marker=marker_style)
assert self.image.is_marking
assert self.image.marker == marker_style
assert not self.image.click_center
assert not self.image.click_drag

# scroll_pan better activate when marking otherwise there is
# no way to pan while interactively marking
assert self.image.scroll_pan

# Make sure that when we stop_marking we get our old controls back.
self.image.stop_marking()
assert self.image.click_center
assert not self.image.click_drag
assert not self.image.scroll_pan

# Make sure no warning is issued when trying to retrieve markers
# with a name that does not exist.
with warnings.catch_warnings():
warnings.simplefilter("error")
t = self.image.get_markers(marker_name='markymark')

self._assert_empty_marker_table(t)

self.image.click_drag = True
self.image.start_marking()
assert not self.image.click_drag

# Add a marker to the interactive marking table
self.image.add_markers(
Table(data=[[50], [50]], names=['x', 'y'], dtype=('float', 'float')),
marker_name=self.image.DEFAULT_INTERACTIVE_MARKER_NAME,
)
assert self._get_marker_names_as_set() == set([self.image.DEFAULT_INTERACTIVE_MARKER_NAME])

# Clear markers to not pollute other tests.
self.image.stop_marking(clear_markers=True)

assert self.image.is_marking is False
self._assert_empty_marker_table(self.image.get_markers(marker_name="all"))

# Hate this, should add to public API
marknames = self._get_marker_names_as_set()
assert len(marknames) == 0

# Make sure that click_drag is restored as expected
assert self.image.click_drag
# TODO: add test that checks that retrieving markers with an unknown name issues no error

def test_add_markers(self):
original_marker_name = self.image.DEFAULT_MARKER_NAME
Expand Down Expand Up @@ -391,29 +329,21 @@ def test_click_drag(self):

# If is_marking is true then trying to enable click_drag should fail
self.image.click_drag = False
self.image.start_marking()
with pytest.raises(ValueError, match=r'([Ii]nteractive marking)|(while in marking mode)|(while marking is active)'):
self.image.click_drag = True
self.image.stop_marking()

def test_click_center(self):
# Set this to ensure that click_center turns it off
self.image.click_drag = True

# Make sure that setting click_center to False does not turn off
# click_draf.
# click_drag.
self.image.click_center = False
assert self.image.click_drag

self.image.click_center = True
assert not self.image.click_drag

self.image.click_center = False
# If is_marking is true then trying to enable click_center should fail
self.image.start_marking()
with pytest.raises(ValueError, match=r'([Ii]nteractive marking)|(while marking is active)'):
self.image.click_center = True
self.image.stop_marking()


def test_scroll_pan(self):
# Make sure scroll_pan is actually settable
Expand Down