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

Provide stronger checking for region display. Fix x/y flip. #2465

Merged
merged 1 commit into from
Dec 7, 2023
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
3 changes: 2 additions & 1 deletion glue/tests/visual/py311-test-visual.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"glue.viewers.histogram.tests.test_viewer.test_simple_viewer": "cb08123fbad135ab614bb7ec13475fcc83321057d884fe80c3a32970b2d14762",
"glue.viewers.image.tests.test_viewer.test_simple_viewer": "72abd60b484d14f721254f027bb0ab9b36245d5db77eb87693f4dd9998fd28be",
"glue.viewers.image.tests.test_viewer.test_region_layer": "b54088d1f31562d309d35c5e90c0a382c7ba4c16cc0db8134e9532f71f047076",
"glue.viewers.image.tests.test_viewer.test_region_layer": "0114922ab0a3980f56252656c69545927841aea0e6950250cdc2b1bafcd19d50",
"glue.viewers.image.tests.test_viewer.test_region_layer_flip": "a142142f34961aba7e98188ad43abafe0e6e5b82e13e8cdab5131d297ed5832c",
"glue.viewers.profile.tests.test_viewer.test_simple_viewer": "f68a21be5080fec513388b2d2b220512e7b0df5498e2489da54e58708de435b3",
"glue.viewers.scatter.tests.test_viewer.test_simple_viewer": "1020a7bd3abe40510b9e03047c3b423b75c3c64ac18e6dcd6257173cec1ed53f",
"glue.viewers.scatter.tests.test_viewer.test_scatter_density_map": "3379d655262769a6ccbdbaf1970bffa9237adbec23a93d3ab75da51b9a3e7f8b"
Expand Down
40 changes: 38 additions & 2 deletions glue/viewers/image/tests/test_viewer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import numpy as np

from echo import delay_callback
from glue.tests.visual.helpers import visual_test
from glue.viewers.image.viewer import SimpleImageViewer
from glue.core.application_base import Application
Expand Down Expand Up @@ -58,6 +58,42 @@ def test_region_layer():
viewer.add_data(image_data)
viewer.add_data(region_data)

app.data_collection.new_subset_group(label='subset1', subset_state=region_data.id['values'] > 2)
return viewer.figure


@visual_test
def test_region_layer_flip():
poly_1 = Polygon([(20, 20), (60, 20), (60, 40), (20, 40)])
poly_2 = Polygon([(60, 50), (60, 70), (80, 70), (80, 50)])
poly_3 = Polygon([(10, 10), (15, 10), (15, 15), (10, 15)])
poly_4 = Polygon([(10, 20), (15, 20), (15, 30), (10, 30), (12, 25)])

polygons = MultiPolygon([poly_3, poly_4])

geoms = np.array([poly_1, poly_2, polygons])
values = np.array([1, 2, 3])
region_data = RegionData(regions=geoms, values=values)

image_data = Data(x=np.arange(10000).reshape((100, 100)), label='data1')
app = Application()
app.data_collection.append(image_data)
app.data_collection.append(region_data)

link1 = LinkSame(region_data.center_x_id, image_data.pixel_component_ids[0])
link2 = LinkSame(region_data.center_y_id, image_data.pixel_component_ids[1])
app.data_collection.add_link(link1)
app.data_collection.add_link(link2)

viewer = app.new_data_viewer(SimpleImageViewer)
viewer.add_data(image_data)
viewer.add_data(region_data)

# We need this delay callback here because, while this works in the QT GUI,
# we need to make sure not to try and redraw the regions while we are flipping
# the coordinates displayed.

with delay_callback(viewer.state, 'x_att', 'y_att'):
viewer.state.x_att = image_data.pixel_component_ids[0]
viewer.state.y_att = image_data.pixel_component_ids[1]

return viewer.figure
60 changes: 51 additions & 9 deletions glue/viewers/scatter/layer_artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@
(not data.linked_to_center_comp(self._viewer_state.x_att_world))):
raise IncompatibleAttribute
x = ensure_numerical(self.layer[self._viewer_state.x_att].ravel())
xx = ensure_numerical(data[data.center_x_id].ravel())

Check warning on line 645 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L645

Added line #L645 was not covered by tests
except (IncompatibleAttribute, IndexError):
# The following includes a call to self.clear()
self.disable_invalid_attributes(self._viewer_state.x_att)
Expand All @@ -655,30 +656,63 @@
(not data.linked_to_center_comp(self._viewer_state.y_att_world))):
raise IncompatibleAttribute
y = ensure_numerical(self.layer[self._viewer_state.y_att].ravel())
yy = ensure_numerical(data[data.center_y_id].ravel())

Check warning on line 659 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L659

Added line #L659 was not covered by tests
except (IncompatibleAttribute, IndexError):
# The following includes a call to self.clear()
self.disable_invalid_attributes(self._viewer_state.y_att)
return
else:
self.enable()

# We need to make sure that x and y viewer attributes are
# really the center_x and center_y attributes of the underlying
# data, so we compare the values on the centroids using the
# glue data access machinery.

regions = self.layer[region_att]

def flip_xy(g):
return transform(lambda x, y: (y, x), g)

Check warning on line 675 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L674-L675

Added lines #L674 - L675 were not covered by tests

x_no_match = False
if np.array_equal(y, yy):
if np.array_equal(x, xx):
self.enable()

Check warning on line 680 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L677-L680

Added lines #L677 - L680 were not covered by tests
else:
x_no_match = True

Check warning on line 682 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L682

Added line #L682 was not covered by tests
else:
if np.array_equal(y, xx) and np.array_equal(x, yy): # This means x and y have been swapped
regions = [flip_xy(g) for g in regions]
self.enable()

Check warning on line 686 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L684-L686

Added lines #L684 - L686 were not covered by tests
else:
self.disable_invalid_attributes(self._viewer_state.y_att)
if x_no_match:
self.disable_invalid_attributes(self._viewer_state.x_att)
return

Check warning on line 691 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L688-L691

Added lines #L688 - L691 were not covered by tests

# If we are using world coordinates (i.e. the regions are specified in world coordinates)
# we need to transform the geometries of the regions into pixel coordinates for display
# Note that this calls a custom version of the transform function from shapely
# to accomodate glue WCS objects
if self._viewer_state._display_world:
# First, convert to world coordinates
tfunc = data.get_transform_to_cids([self._viewer_state.x_att_world, self._viewer_state.y_att_world])
regions = np.array([transform(tfunc, g) for g in regions])
try:
tfunc = data.get_transform_to_cids([self._viewer_state.x_att_world, self._viewer_state.y_att_world])
regions = np.array([transform(tfunc, g) for g in regions])

Check warning on line 701 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L699-L701

Added lines #L699 - L701 were not covered by tests

# Then convert to pixels for display
world2pix = self._viewer_state.reference_data.coords.world_to_pixel_values
regions = np.array([transform_shapely(world2pix, g) for g in regions])
# Then convert to pixels for display
world2pix = self._viewer_state.reference_data.coords.world_to_pixel_values
regions = np.array([transform_shapely(world2pix, g) for g in regions])
except ValueError:
self.disable_invalid_attributes([self._viewer_state.x_att_world, self._viewer_state.y_att_world])
return

Check warning on line 708 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L704-L708

Added lines #L704 - L708 were not covered by tests
else:
tfunc = data.get_transform_to_cids([self._viewer_state.x_att, self._viewer_state.y_att])
regions = np.array([transform(tfunc, g) for g in regions])
try:
tfunc = data.get_transform_to_cids([self._viewer_state.x_att, self._viewer_state.y_att])
regions = np.array([transform(tfunc, g) for g in regions])
except ValueError:
self.disable_invalid_attributes([self._viewer_state.x_att, self._viewer_state.y_att])
return

Check warning on line 715 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L710-L715

Added lines #L710 - L715 were not covered by tests

# decompose GeometryCollections
geoms, multiindex = _sanitize_geoms(regions, prefix="Geom")
Expand Down Expand Up @@ -753,8 +787,16 @@
full_sphere = getattr(self._viewer_state, 'using_full_sphere', False)
change_from_limits = full_sphere and len(changed & LIMIT_PROPERTIES) > 0
if force or change_from_limits or len(changed & DATA_PROPERTIES) > 0:
self._update_data()
force = True
# This is the signature of flipping the x and y axis of an image in the UI
# We must *not* run update_data right away, or it will return an error
# The delay_callback wrapper on state._on_xatt_world_change() and
# state._on_yatt_world_change() is not properly deferring the callback
# until after x and y have been fully swapped, so we need to do it manually.
if changed == {'x_att_world', 'x_att', 'y_att_world'} or changed == {'y_att_world', 'y_att', 'x_att_world'}:
pass

Check warning on line 796 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L795-L796

Added lines #L795 - L796 were not covered by tests
else:
self._update_data()
force = True

Check warning on line 799 in glue/viewers/scatter/layer_artist.py

View check run for this annotation

Codecov / codecov/patch

glue/viewers/scatter/layer_artist.py#L798-L799

Added lines #L798 - L799 were not covered by tests

if force or len(changed & VISUAL_PROPERTIES) > 0:
self._update_visual_attributes(changed, force=force)
Expand Down