INFO: Subset API audit 2/4 (get, export, write)

[pllim]

subset_tools.get_regions(): We want this to be the canonical API.

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

Lines 182 to 183 in 4ffc3a0

	def get_regions(self, region_type=None, list_of_subset_labels=None,
	use_display_units=False):

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

Lines 207 to 212 in 4ffc3a0

	Returns
	-------
	regions : dict
	A dictionary mapping subset labels to their respective ``regions``
	objects (for spatial regions) or ``SpectralRegions`` objects
	(for spectral regions).

jdaviz.app.get_subsets(): We want to be able to have subset_tools.get_regions() call this instead 🐱, but somehow it was not possible, why? Clare said it was because of composite subsets. Note that this method is way more complicated than the one in subset_tools and calls another recursive method and also is more powerful.

jdaviz/jdaviz/app.py

Lines 921 to 924 in 4ffc3a0

	def get_subsets(self, subset_name=None, spectral_only=False,
	spatial_only=False, object_only=False,
	simplify_spectral=True, use_display_units=False,
	include_sky_region=False):

jdaviz/jdaviz/app.py

Lines 952 to 957 in 4ffc3a0

	Returns
	-------
	data : dict
	Returns a nested dictionary with, for each subset, 'name', 'glue_state',
	'region', 'sky_region' (set to None if not applicable), and 'subset_state'.
	If subset is composite, each constituant subset will be included individually.

jdaviz.app.get_sub_regions(): Only used internally in jdaviz.app.get_subsets(). Keep an eye out in case it is no longer necessary if we remove jdaviz.app.get_subsets() (which probably won't happen).

jdaviz/jdaviz/app.py

Lines 1138 to 1139 in 4ffc3a0

	def get_sub_regions(self, subset_state, simplify_spectral=True,
	use_display_units=False, get_sky_regions=False):

export.save_subset_as_region() and export.save_subset_as_table(): Relies on jdaviz.app.get_subsets(). Should this use subset_tools API instead? No, Kyle said it's weird to have plugin use another plugin's API, so calling app level methods internally is fine.

jdaviz/jdaviz/configs/default/plugins/export/export.py

Line 731 in 4ffc3a0

def save_subset_as_region(self, selected_subset_label, filename):

jdaviz/jdaviz/configs/default/plugins/export/export.py

Line 751 in 4ffc3a0

def save_subset_as_table(self, filename):

subset_tools._unpack_get_subsets_for_ui(): Relies on jdaviz.app.get_subsets(). Should this use subset_tools API instead? No, see above.

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

Line 350 in 4ffc3a0

def _unpack_get_subsets_for_ui(self):

jdaviz.core.region_translators._get_region_from_spatial_subset(): It is used internally in jdaviz.core.template_mixin.SubsetSelect._get_spatial_region(), aper_phot._background_selected_changed(), and subset_tools.recenter(). Should this function be retired? Can it be replaced by subset_tools API? 🐱

jdaviz/jdaviz/core/region_translators.py

Line 23 in 4ffc3a0

def _get_region_from_spatial_subset(plugin_obj, subset_state):

Already deprecated

🐱

jdaviz/jdaviz/core/helpers.py

Lines 792 to 793 in 4ffc3a0

	@deprecated(since="4.1", alternative="subset_tools.get_regions")
	def get_interactive_regions(self):

jdaviz/jdaviz/configs/specviz/helper.py

Lines 151 to 152 in 4ffc3a0

	@deprecated(since="4.1", alternative="subset_tools.get_regions")
	def get_spectral_regions(self, use_display_units=False):

🐱

[pllim]

Done. Open to comments.