Skip to content

Commit 190834f

Browse files
Backport PR #4083: Programmatically evaluate subsets to avoid creating new subset copies when adjusting (#4127)
Co-authored-by: Matthew Portman <29168957+MatthewPortman@users.noreply.github.com>
1 parent 5b3ea65 commit 190834f

File tree

5 files changed

+665
-118
lines changed

5 files changed

+665
-118
lines changed

CHANGES.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
Bug Fixes
55
---------
66

7+
- Fixed bug where creating new subsets in a fresh app instance and adjusting them creates a copy
8+
instead of adjusting the original subset. [#4083]
9+
710
Cubeviz
811
^^^^^^^
912

jdaviz/configs/default/plugins/subset_tools/subset_tools.py

Lines changed: 124 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,136 +1283,143 @@ def _load_regions(self, regions, edit_subset=None, combination_mode=None, max_nu
12831283
previous_mode = self.app.session.edit_subset_mode.mode
12841284
previous_subset = self.app.session.edit_subset_mode.edit_subset
12851285

1286-
with self.app._jdaviz_helper.batch_load():
1287-
# This method can edit a particular subset or create a new subset
1288-
# and apply the combination modes depending on this argument
1289-
if edit_subset and combination_mode is not None:
1290-
self.subset.selected = edit_subset
1291-
elif edit_subset and combination_mode is None:
1292-
# If combination_mode is not set, assume the user
1293-
# wants to update the subset in edit_subset
1294-
self.subset.selected = edit_subset
1295-
combination_mode = 'replace'
1296-
else:
1297-
# self.app.session.edit_subset_mode.edit_subset = None
1298-
self.subset.selected = self.subset.default_text
1299-
1300-
label_index = 0
1301-
for index, region in enumerate(regions):
1302-
# Set combination mode for how region will be applied to current subset
1303-
# or created as a new subset
1304-
if combo_mode_is_list:
1305-
combo_mode = combination_mode[index]
1286+
self.app._importing_regions = True
1287+
try:
1288+
with self.app._jdaviz_helper.batch_load():
1289+
# This method can edit a particular subset or create a new subset
1290+
# and apply the combination modes depending on this argument
1291+
if edit_subset and combination_mode is not None:
1292+
self.subset.selected = edit_subset
1293+
elif edit_subset and combination_mode is None:
1294+
# If combination_mode is not set, assume the user
1295+
# wants to update the subset in edit_subset
1296+
self.subset.selected = edit_subset
1297+
combination_mode = 'replace'
13061298
else:
1307-
combo_mode = combination_mode
1308-
1309-
# Combination_mode should be 'new' if combo_mode is not set or explicitly 'new'
1310-
if combo_mode == 'new' or combo_mode is None:
1311-
self.combination_mode.selected = 'new'
1312-
elif combo_mode:
1313-
self.combination_mode.selected = combo_mode
1314-
1315-
if (isinstance(region, (SkyCircularAperture, SkyEllipticalAperture,
1316-
SkyRectangularAperture, SkyCircularAnnulus,
1317-
CircleSkyRegion, EllipseSkyRegion,
1318-
RectangleSkyRegion, CircleAnnulusSkyRegion))
1319-
and not has_wcs):
1320-
bad_regions.append((region, 'Sky region provided but data has no valid WCS'))
1321-
continue
1299+
# self.app.session.edit_subset_mode.edit_subset = None
1300+
self.subset.selected = self.subset.default_text
1301+
1302+
label_index = 0
1303+
for index, region in enumerate(regions):
1304+
# Set combination mode for how region will be applied to current subset
1305+
# or created as a new subset
1306+
if combo_mode_is_list:
1307+
combo_mode = combination_mode[index]
1308+
else:
1309+
combo_mode = combination_mode
1310+
1311+
# Combination_mode should be 'new' if combo_mode is not set or explicitly 'new'
1312+
if combo_mode == 'new' or combo_mode is None:
1313+
self.combination_mode.selected = 'new'
1314+
elif combo_mode:
1315+
self.combination_mode.selected = combo_mode
1316+
1317+
if (isinstance(region, (SkyCircularAperture, SkyEllipticalAperture,
1318+
SkyRectangularAperture, SkyCircularAnnulus,
1319+
CircleSkyRegion, EllipseSkyRegion,
1320+
RectangleSkyRegion, CircleAnnulusSkyRegion))
1321+
and not has_wcs):
1322+
bad_regions.append(
1323+
(region, 'Sky region provided but data has no valid WCS'))
1324+
continue
13221325

1323-
if (isinstance(region, (CircularAperture, EllipticalAperture,
1324-
RectangularAperture, CircularAnnulus,
1325-
CirclePixelRegion, EllipsePixelRegion,
1326-
RectanglePixelRegion, CircleAnnulusPixelRegion))
1327-
and (hasattr(self.app, '_link_type') and self.app._link_type == "wcs")):
1328-
bad_regions.append((region, 'Pixel region provided but data is aligned by WCS'))
1329-
continue
1326+
if (isinstance(region, (CircularAperture, EllipticalAperture,
1327+
RectangularAperture, CircularAnnulus,
1328+
CirclePixelRegion, EllipsePixelRegion,
1329+
RectanglePixelRegion, CircleAnnulusPixelRegion))
1330+
and (hasattr(self.app, '_link_type') and self.app._link_type == "wcs")):
1331+
bad_regions.append(
1332+
(region, 'Pixel region provided but data is aligned by WCS'))
1333+
continue
13301334

1331-
# photutils: Convert to region shape first
1332-
if isinstance(region, (CircularAperture, SkyCircularAperture,
1333-
EllipticalAperture, SkyEllipticalAperture,
1334-
RectangularAperture, SkyRectangularAperture,
1335-
CircularAnnulus, SkyCircularAnnulus)):
1336-
region = aperture2regions(region)
1337-
1338-
# region: Convert to ROI.
1339-
# NOTE: Out-of-bounds ROI will succeed; this is native glue behavior.
1340-
if (isinstance(region, (CirclePixelRegion, CircleSkyRegion,
1341-
EllipsePixelRegion, EllipseSkyRegion,
1342-
RectanglePixelRegion, RectangleSkyRegion,
1343-
CircleAnnulusPixelRegion, CircleAnnulusSkyRegion))):
1344-
try:
1345-
if getattr(data.coords, 'world_n_dim', None) == 3:
1346-
data_wcs = _get_celestial_wcs(data.coords)
1347-
state = regions2roi(region, wcs=data_wcs)
1348-
else:
1349-
state = regions2roi(region, wcs=data.coords)
1350-
except ValueError:
1351-
if '_orig_spatial_wcs' not in data.meta:
1352-
bad_regions.append((region,
1353-
f'Failed to load: _orig_spatial_wcs'
1354-
f' meta tag not in {data.label}'))
1355-
continue
1356-
state = regions2roi(region, wcs=data.meta['_orig_spatial_wcs'])
1357-
viewer.apply_roi(state)
1358-
1359-
elif isinstance(region, (CircularROI, CircularAnnulusROI,
1360-
EllipticalROI, RectangularROI)):
1361-
viewer.apply_roi(region)
1362-
1363-
elif isinstance(region, SpectralRegion):
1364-
# Use viewer_name if provided in kwarg, otherwise use
1365-
# default spectrum viewer name
1366-
range_viewer = self.app.get_viewer(viewer_parameter) if viewer_parameter else self.spectrum_viewer # noqa
1367-
s = RangeSubsetState(lo=region.lower.value, hi=region.upper.value,
1368-
att=range_viewer.state.x_att)
1369-
range_viewer.apply_subset_state(s)
1370-
1371-
# Last resort: Masked Subset that is static (if data is not a cube)
1372-
elif data.ndim == 2:
1373-
im = None
1374-
if hasattr(region, 'to_pixel'): # Sky region: Convert to pixel region
1375-
if not has_wcs:
1376-
bad_regions.append((region, 'Sky region provided but data has no valid WCS')) # noqa
1335+
# photutils: Convert to region shape first
1336+
if isinstance(region, (CircularAperture, SkyCircularAperture,
1337+
EllipticalAperture, SkyEllipticalAperture,
1338+
RectangularAperture, SkyRectangularAperture,
1339+
CircularAnnulus, SkyCircularAnnulus)):
1340+
region = aperture2regions(region)
1341+
1342+
# region: Convert to ROI.
1343+
# NOTE: Out-of-bounds ROI will succeed; this is native glue behavior.
1344+
if (isinstance(region, (CirclePixelRegion, CircleSkyRegion,
1345+
EllipsePixelRegion, EllipseSkyRegion,
1346+
RectanglePixelRegion, RectangleSkyRegion,
1347+
CircleAnnulusPixelRegion, CircleAnnulusSkyRegion))):
1348+
try:
1349+
if getattr(data.coords, 'world_n_dim', None) == 3:
1350+
data_wcs = _get_celestial_wcs(data.coords)
1351+
state = regions2roi(region, wcs=data_wcs)
1352+
else:
1353+
state = regions2roi(region, wcs=data.coords)
1354+
except ValueError:
1355+
if '_orig_spatial_wcs' not in data.meta:
1356+
bad_regions.append((region,
1357+
f'Failed to load: _orig_spatial_wcs'
1358+
f' meta tag not in {data.label}'))
1359+
continue
1360+
state = regions2roi(region, wcs=data.meta['_orig_spatial_wcs'])
1361+
viewer.apply_roi(state)
1362+
1363+
elif isinstance(region, (CircularROI, CircularAnnulusROI,
1364+
EllipticalROI, RectangularROI)):
1365+
viewer.apply_roi(region)
1366+
1367+
elif isinstance(region, SpectralRegion):
1368+
# Use viewer_name if provided in kwarg, otherwise use
1369+
# default spectrum viewer name
1370+
range_viewer = self.app.get_viewer(viewer_parameter) if viewer_parameter else self.spectrum_viewer # noqa
1371+
s = RangeSubsetState(lo=region.lower.value, hi=region.upper.value,
1372+
att=range_viewer.state.x_att)
1373+
range_viewer.apply_subset_state(s)
1374+
1375+
# Last resort: Masked Subset that is static (if data is not a cube)
1376+
elif data.ndim == 2:
1377+
im = None
1378+
if hasattr(region, 'to_pixel'): # Sky region: Convert to pixel region
1379+
if not has_wcs:
1380+
bad_regions.append((region, 'Sky region provided but data has no valid WCS')) # noqa
1381+
continue
1382+
region = region.to_pixel(data.coords)
1383+
1384+
if hasattr(region, 'to_mask'):
1385+
try:
1386+
mask = region.to_mask(**kwargs)
1387+
im = mask.to_image(data.shape) # Can be None
1388+
except Exception as e: # pragma: no cover
1389+
bad_regions.append((region, f'Failed to load: {repr(e)}'))
1390+
continue
1391+
1392+
# Boolean mask as input is supported but not advertised.
1393+
elif (isinstance(region, np.ndarray) and region.shape == data.shape
1394+
and region.dtype == np.bool_):
1395+
im = region
1396+
1397+
if im is None:
1398+
bad_regions.append((region, 'Mask creation failed'))
13771399
continue
1378-
region = region.to_pixel(data.coords)
13791400

1380-
if hasattr(region, 'to_mask'):
1401+
# NOTE: Region creation info is thus lost.
13811402
try:
1382-
mask = region.to_mask(**kwargs)
1383-
im = mask.to_image(data.shape) # Can be None
1403+
mask_label = f'{msg_prefix} {msg_count}'
1404+
state = MaskSubsetState(im, data.pixel_component_ids)
1405+
self.app.data_collection.new_subset_group(mask_label, state)
1406+
msg_count += 1
13841407
except Exception as e: # pragma: no cover
13851408
bad_regions.append((region, f'Failed to load: {repr(e)}'))
13861409
continue
1387-
1388-
# Boolean mask as input is supported but not advertised.
1389-
elif (isinstance(region, np.ndarray) and region.shape == data.shape
1390-
and region.dtype == np.bool_):
1391-
im = region
1392-
1393-
if im is None:
1410+
else:
13941411
bad_regions.append((region, 'Mask creation failed'))
13951412
continue
1413+
n_loaded += 1
1414+
if max_num_regions is not None and n_loaded >= max_num_regions:
1415+
break
13961416

1397-
# NOTE: Region creation info is thus lost.
1398-
try:
1399-
mask_label = f'{msg_prefix} {msg_count}'
1400-
state = MaskSubsetState(im, data.pixel_component_ids)
1401-
self.app.data_collection.new_subset_group(mask_label, state)
1402-
msg_count += 1
1403-
except Exception as e: # pragma: no cover
1404-
bad_regions.append((region, f'Failed to load: {repr(e)}'))
1405-
continue
1406-
else:
1407-
bad_regions.append((region, 'Mask creation failed'))
1408-
continue
1409-
n_loaded += 1
1410-
if max_num_regions is not None and n_loaded >= max_num_regions:
1411-
break
1417+
if self.combination_mode.selected in ('new', 'replace') and subset_label is not None: # noqa
1418+
self.rename_selected(subset_label[label_index])
1419+
label_index += 1
14121420

1413-
if self.combination_mode.selected in ('new', 'replace') and subset_label is not None: # noqa
1414-
self.rename_selected(subset_label[label_index])
1415-
label_index += 1
1421+
finally:
1422+
self.app._importing_regions = False
14161423

14171424
# Revert edit mode and subset to before the import_region call
14181425
self.app.session.edit_subset_mode.edit_subset = previous_subset

0 commit comments

Comments
 (0)