-
Notifications
You must be signed in to change notification settings - Fork 76
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
Imviz: Allow Subset recentering via centroiding #1823
Changes from all commits
d5bdeaf
4ea27bc
ac4e828
404b5c5
f052469
fd7f671
c425f7e
e7c15a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,11 @@ | |
from glue.core.roi import CircularROI, EllipticalROI, RectangularROI | ||
from glue.core.subset import RoiSubsetState, RangeSubsetState, CompositeSubsetState | ||
from glue_jupyter.widgets.subset_mode_vuetify import SelectionModeMenu | ||
from traitlets import List, Unicode, Bool, observe | ||
from traitlets import Any, List, Unicode, Bool, observe | ||
|
||
from jdaviz.core.events import SnackbarMessage | ||
from jdaviz.core.registries import tray_registry | ||
from jdaviz.core.template_mixin import PluginTemplateMixin, SubsetSelect | ||
from jdaviz.core.template_mixin import PluginTemplateMixin, DatasetSelectMixin, SubsetSelect | ||
|
||
__all__ = ['SubsetPlugin'] | ||
|
||
|
@@ -23,7 +23,7 @@ | |
|
||
|
||
@tray_registry('g-subset-plugin', label="Subset Tools") | ||
class SubsetPlugin(PluginTemplateMixin): | ||
class SubsetPlugin(PluginTemplateMixin, DatasetSelectMixin): | ||
template_file = __file__, "subset_plugin.vue" | ||
select = List([]).tag(sync=True) | ||
subset_items = List([]).tag(sync=True) | ||
|
@@ -34,6 +34,8 @@ class SubsetPlugin(PluginTemplateMixin): | |
subset_definitions = List([]).tag(sync=True) | ||
has_subset_details = Bool(False).tag(sync=True) | ||
|
||
subplugins_opened = Any().tag(sync=True) | ||
|
||
is_editable = Bool(False).tag(sync=True) | ||
|
||
def __init__(self, *args, **kwargs): | ||
|
@@ -226,6 +228,141 @@ def vue_update_subset(self, *args): | |
self.hub.broadcast(SnackbarMessage( | ||
f"Failed to update Subset: {repr(err)}", color='error', sender=self)) | ||
|
||
def vue_recenter_subset(self, *args): | ||
# Composite region cannot be edited. This only works for Imviz. | ||
if not self.is_editable or self.config != 'imviz': # no-op | ||
raise NotImplementedError( | ||
f'Cannot recenter: is_editable={self.is_editable}, config={self.config}') | ||
|
||
from photutils.aperture import ApertureStats | ||
from jdaviz.core.region_translators import regions2aperture, _get_region_from_spatial_subset | ||
|
||
try: | ||
reg = _get_region_from_spatial_subset(self, self.subset_selected) | ||
aperture = regions2aperture(reg) | ||
Comment on lines
+241
to
+242
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would it be useful to move this into the component itself for re-use - so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will eventually move upstream. At least Larry or I added a few of these in that table. |
||
data = self.dataset.selected_dc_item | ||
comp = data.get_component(data.main_components[0]) | ||
comp_data = comp.data | ||
phot_aperstats = ApertureStats(comp_data, aperture) | ||
|
||
# Centroid was calculated in selected data, which might or might not be | ||
# the reference data. However, Subset is always defined w.r.t. | ||
# the reference data, so we need to convert back. | ||
viewer = self.app._jdaviz_helper.default_viewer | ||
x, y, _ = viewer._get_real_xy( | ||
data, phot_aperstats.xcentroid, phot_aperstats.ycentroid, reverse=True) | ||
if not np.all(np.isfinite((x, y))): | ||
raise ValueError(f'Invalid centroid ({x}, {y})') | ||
except Exception as err: | ||
self.set_center(self.get_center(), update=False) | ||
self.hub.broadcast(SnackbarMessage( | ||
f"Failed to calculate centroid: {repr(err)}", color='error', sender=self)) | ||
else: | ||
self.set_center((x, y), update=True) | ||
|
||
def get_center(self): | ||
pllim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Return the center of the Subset. | ||
This may or may not be the centroid obtain from data. | ||
|
||
Returns | ||
------- | ||
cen : number, tuple of numbers, or `None` | ||
The center of the Subset in ``x`` or ``(x, y)``, | ||
depending on the Subset type, if applicable. | ||
If Subset is not editable, this returns `None`. | ||
|
||
Raises | ||
------ | ||
NotImplementedError | ||
Subset type is not supported. | ||
|
||
""" | ||
# Composite region cannot be edited. | ||
if not self.is_editable: # no-op | ||
return | ||
pllim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
subset_state = self.subset_select.selected_subset_state | ||
|
||
if isinstance(subset_state, RoiSubsetState): | ||
sbst_obj = subset_state.roi | ||
if isinstance(sbst_obj, (CircularROI, EllipticalROI)): | ||
cen = sbst_obj.get_center() | ||
elif isinstance(sbst_obj, RectangularROI): | ||
cen = sbst_obj.center() | ||
else: # pragma: no cover | ||
raise NotImplementedError( | ||
f'Getting center of {sbst_obj.__class__} is not supported') | ||
|
||
elif isinstance(subset_state, RangeSubsetState): | ||
cen = (subset_state.hi - subset_state.lo) * 0.5 + subset_state.lo | ||
|
||
else: # pragma: no cover | ||
raise NotImplementedError( | ||
f'Getting center of {subset_state.__class__} is not supported') | ||
|
||
return cen | ||
|
||
def set_center(self, new_cen, update=False): | ||
"""Set the desired center for the selected Subset, if applicable. | ||
If Subset is not editable, nothing is done. | ||
|
||
Parameters | ||
---------- | ||
new_cen : number or tuple of numbers | ||
The new center defined either as ``x`` or ``(x, y)``, | ||
depending on the Subset type. | ||
|
||
update : bool | ||
If `True`, the Subset is also moved to the new center. | ||
Otherwise, only the relevant editable fields are updated but the | ||
Subset is not moved. | ||
|
||
Raises | ||
------ | ||
NotImplementedError | ||
Subset type is not supported. | ||
|
||
""" | ||
# Composite region cannot be edited, so just grab first element. | ||
if not self.is_editable: # no-op | ||
return | ||
|
||
subset_state = self.subset_select.selected_subset_state | ||
|
||
if isinstance(subset_state, RoiSubsetState): | ||
x, y = new_cen | ||
sbst_obj = subset_state.roi | ||
if isinstance(sbst_obj, (CircularROI, EllipticalROI)): | ||
self._set_value_in_subset_definition(0, "X Center", "value", x) | ||
self._set_value_in_subset_definition(0, "Y Center", "value", y) | ||
elif isinstance(sbst_obj, RectangularROI): | ||
cx, cy = sbst_obj.center() | ||
dx = x - cx | ||
dy = y - cy | ||
self._set_value_in_subset_definition(0, "Xmin", "value", sbst_obj.xmin + dx) | ||
self._set_value_in_subset_definition(0, "Xmax", "value", sbst_obj.xmax + dx) | ||
self._set_value_in_subset_definition(0, "Ymin", "value", sbst_obj.ymin + dy) | ||
self._set_value_in_subset_definition(0, "Ymax", "value", sbst_obj.ymax + dy) | ||
else: # pragma: no cover | ||
raise NotImplementedError(f'Recentering of {sbst_obj.__class__} is not supported') | ||
|
||
elif isinstance(subset_state, RangeSubsetState): | ||
dx = new_cen - ((subset_state.hi - subset_state.lo) * 0.5 + subset_state.lo) | ||
self._set_value_in_subset_definition(0, "Lower bound", "value", subset_state.lo + dx) | ||
self._set_value_in_subset_definition(0, "Upper bound", "value", subset_state.hi + dx) | ||
|
||
else: # pragma: no cover | ||
raise NotImplementedError( | ||
f'Getting center of {subset_state.__class__} is not supported') | ||
|
||
if update: | ||
self.vue_update_subset() | ||
else: | ||
# Force UI to update on browser without changing the subset. | ||
tmp = self.subset_definitions | ||
self.subset_definitions = [] | ||
self.subset_definitions = tmp | ||
|
||
# List of JSON-like dict is nice for front-end but a pain to look up, | ||
# so we use these helper functions. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,29 @@ | |
</v-col> | ||
</v-row> | ||
|
||
<!-- Sub-plugin for recentering of spatial subset (Imviz only) --> | ||
<v-row v-if="config=='imviz' && is_editable"> | ||
<v-expansion-panels accordion v-model="subplugins_opened"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Magical. Thanks, Kyle! |
||
<v-expansion-panel> | ||
<v-expansion-panel-header > | ||
<span style="padding: 6px">Recenter</span> | ||
</v-expansion-panel-header> | ||
<v-expansion-panel-content> | ||
<plugin-dataset-select | ||
:items="dataset_items" | ||
:selected.sync="dataset_selected" | ||
:show_if_single_entry="true" | ||
label="Data" | ||
hint="Select the data for centroiding." | ||
/> | ||
<v-row justify="end" no-gutters> | ||
<v-btn color="primary" text @click="recenter_subset">Recenter</v-btn> | ||
</v-row> | ||
</v-expansion-panel-content> | ||
</v-expansion-panel> | ||
</v-expansion-panels> | ||
</v-row> | ||
|
||
<!-- Composite region cannot be edited, so just grab first element. --> | ||
<div v-if="is_editable"> | ||
<v-row v-for="(item, index2) in subset_definitions[0]" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
from regions import PixCoord, CirclePixelRegion | ||
|
||
from jdaviz.configs.imviz.tests.utils import BaseImviz_WCS_GWCS | ||
|
||
|
||
class TestImvizSpatialSubsetCentroid(BaseImviz_WCS_GWCS): | ||
def test_centroiding(self): | ||
reg = CirclePixelRegion(PixCoord(2, 2), 3) | ||
self.imviz.load_regions(reg) | ||
|
||
plg = self.imviz.plugins['Subset Tools'] | ||
plg._obj.subset_selected = 'Subset 1' | ||
|
||
# Since they are linked by pixels, the bright corner pixel aligns | ||
# and nothing should change. | ||
for data_label in ('fits_wcs[DATA]', 'gwcs[DATA]', 'no_wcs'): | ||
plg._obj.dataset_selected = data_label | ||
plg._obj.set_center((2, 2), update=True) # Move the Subset back first. | ||
plg._obj.vue_recenter_subset() | ||
|
||
# Calculate and move to centroid. | ||
for key in ("X Center", "Y Center"): | ||
assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1 | ||
assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1 | ||
|
||
# Radius will not be touched. | ||
for key in ("value", "orig"): | ||
assert plg._obj._get_value_from_subset_definition(0, "Radius", key) == 3 | ||
|
||
assert plg._obj.get_center() == (-1, -1) | ||
|
||
# FITS WCS and GWCS are rotated from each other. | ||
# Plain array always by pixel wrt FITS WCS. | ||
self.imviz.link_data(link_type='wcs') | ||
|
||
plg._obj.dataset_selected = 'fits_wcs[DATA]' | ||
plg._obj.set_center((2, 2), update=True) # Move the Subset back first. | ||
plg._obj.vue_recenter_subset() | ||
for key in ("X Center", "Y Center"): | ||
assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1 | ||
assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1 | ||
|
||
# GWCS does not extrapolate and this Subset is out of bounds, | ||
# so will get NaNs and enter the exception handling logic. | ||
plg._obj.dataset_selected = 'gwcs[DATA]' | ||
plg._obj.set_center((2, 2), update=True) # Move the Subset back first. | ||
plg._obj.vue_recenter_subset() | ||
for key in ("X Center", "Y Center"): | ||
assert plg._obj._get_value_from_subset_definition(0, key, "value") == 2 | ||
assert plg._obj._get_value_from_subset_definition(0, key, "orig") == 2 | ||
|
||
# No WCS case will be same as FITS WCS. | ||
plg._obj.dataset_selected = 'no_wcs' | ||
plg._obj.vue_recenter_subset() | ||
for key in ("X Center", "Y Center"): | ||
assert plg._obj._get_value_from_subset_definition(0, key, "value") == -1 | ||
assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1 | ||
|
||
# This ends up not getting used in production but just in case we need it | ||
# back in the future. | ||
plg._obj.set_center((2, 2), update=False) | ||
for key in ("X Center", "Y Center"): | ||
assert plg._obj._get_value_from_subset_definition(0, key, "value") == 2 | ||
assert plg._obj._get_value_from_subset_definition(0, key, "orig") == -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there an issue filed for fixing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember, maybe @rosteen does. It only happens if you load things like Space Invader (which turns into masked subset), so 99% of users will never even encounter this.