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

catalog search handle from file edge case and implement max sources #3337

Merged
merged 12 commits into from
Dec 13, 2024
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ Imviz

* Add Roman WFI and CGI footprints to the Footprints plugin. [#3322]

- Catalog Search plugin now exposes a maximum sources limit for all catalogs and resolves an edge case
when loading a catalog from a file that only contains one source. [#3337]

Mosviz
^^^^^^

Expand Down
56 changes: 41 additions & 15 deletions jdaviz/configs/imviz/plugins/catalogs/catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
catalog_selected = Unicode("").tag(sync=True)
results_available = Bool(False).tag(sync=True)
number_of_results = Int(0).tag(sync=True)
max_gaia_sources = IntHandleEmpty(1000).tag(sync=True)
max_sources = IntHandleEmpty(1000).tag(sync=True)

# setting the default table headers and values
_default_table_values = {
Expand Down Expand Up @@ -143,6 +143,8 @@
# the distance between the longest zoom limits and the center point
zoom_radius = max(skycoord_center.separation(zoom_coordinate))

max_sources_used = False

# conducts search based on SDSS
if self.catalog_selected == "SDSS":
from astroquery.sdss import SDSS
Expand All @@ -159,6 +161,9 @@
zoom_radius = r_max
query_region_result = SDSS.query_region(skycoord_center, radius=zoom_radius,
data_release=17)
if len(query_region_result) > self.max_sources:
query_region_result = query_region_result[:self.max_sources]
Comment on lines +164 to +165
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have a warning for this (maybe in-UI)?

Copy link
Contributor Author

@gibsongreen gibsongreen Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a snackbar message instead of a warning since the user manually enters the max_sources themselves. Also added it for the Gaia (and SDSS) catalog with the custom logic for this.

max_sources_used = True
except Exception as e: # nosec
errmsg = (f"Failed to query {self.catalog_selected} with c={skycoord_center} and "
f"r={zoom_radius}: {repr(e)}")
Expand All @@ -182,11 +187,14 @@
unit='deg')

elif self.catalog_selected == 'Gaia':
from astroquery.gaia import Gaia, conf

with conf.set_temp("ROW_LIMIT", self.max_gaia_sources):
sources = Gaia.query_object(skycoord_center, radius=zoom_radius,
columns=('source_id', 'ra', 'dec'))
from astroquery.gaia import Gaia

Gaia.ROW_LIMIT = self.max_sources
sources = Gaia.query_object(skycoord_center, radius=zoom_radius,
columns=('source_id', 'ra', 'dec')
)
if len(sources) == self.max_sources:
max_sources_used = True
self.app._catalog_source_table = sources
skycoord_table = SkyCoord(sources['ra'], sources['dec'], unit='deg')

Expand All @@ -195,8 +203,11 @@
# but this exceptions might be raised here if setting from_file from the UI
table = self.catalog.selected_obj
self.app._catalog_source_table = table
skycoord_table = table['sky_centroid']

if len(table['sky_centroid']) > self.max_sources:
skycoord_table = table['sky_centroid'][:self.max_sources]
gibsongreen marked this conversation as resolved.
Show resolved Hide resolved
max_sources_used = True
else:
skycoord_table = table['sky_centroid']
else:
self.results_available = False
self.number_of_results = 0
Expand All @@ -209,6 +220,13 @@
self.app._catalog_source_table = None
return

if max_sources_used:
snackbar_message = SnackbarMessage(
f"{self.catalog_selected} queried, results returned were limited using max_sources = {self.max_sources}.", # noqa
color="success",
sender=self)
self.hub.broadcast(snackbar_message)

# coordinates found are converted to pixel coordinates
pixel_table = viewer.state.reference_data.coords.world_to_pixel(skycoord_table)
# coordinates are filtered out (using a mask) if outside the zoom range
Expand All @@ -225,6 +243,11 @@
y_coordinates = np.squeeze(filtered_pair_pixel_table[1])

if self.catalog_selected in ["SDSS", "Gaia"]:
# for single source convert table information to lists for zipping
if len(self.app._catalog_source_table) == 1 or self.max_sources == 1:
x_coordinates = [x_coordinates]
y_coordinates = [y_coordinates]

Check warning on line 249 in jdaviz/configs/imviz/plugins/catalogs/catalogs.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/imviz/plugins/catalogs/catalogs.py#L248-L249

Added lines #L248 - L249 were not covered by tests

for row, x_coord, y_coord in zip(self.app._catalog_source_table,
x_coordinates, y_coordinates):
if self.catalog_selected == "SDSS":
Expand All @@ -236,23 +259,26 @@
'Declination (degrees)': row['dec'],
'Object ID': row_id.astype(str),
'id': len(self.table),
'x_coord': x_coord,
'y_coord': y_coord}
'x_coord': x_coord.item() if x_coord.size == 1 else x_coord,
'y_coord': y_coord.item() if y_coord.size == 1 else y_coord}
self.table.add_item(row_info)

# NOTE: If performance becomes a problem, see
# https://docs.astropy.org/en/stable/table/index.html#performance-tips
if self.catalog_selected == 'From File...':
elif self.catalog_selected in ["From File..."]:
# for single source convert table information to lists for zipping
if len(self.app._catalog_source_table) == 1 or self.max_sources == 1:
x_coordinates = [x_coordinates]
y_coordinates = [y_coordinates]

for row, x_coord, y_coord in zip(self.app._catalog_source_table,
x_coordinates, y_coordinates):
# Check if the row contains the required keys
row_info = {'Right Ascension (degrees)': row['sky_centroid'].ra.deg,
'Declination (degrees)': row['sky_centroid'].dec.deg,
'Object ID': str(row.get('label', 'N/A')),
'id': len(self.table),
'x_coord': x_coord,
'y_coord': y_coord}

'x_coord': x_coord.item() if x_coord.size == 1 else x_coord,
'y_coord': y_coord.item() if y_coord.size == 1 else y_coord}
self.table.add_item(row_info)

filtered_skycoord_table = viewer.state.reference_data.coords.pixel_to_world(x_coordinates,
Expand Down
9 changes: 6 additions & 3 deletions jdaviz/configs/imviz/plugins/catalogs/catalogs.vue
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@
</j-docs-link>
</v-row>

<v-row v-if="catalog_selected === 'Gaia'">
<v-row>
<v-text-field
v-model.number="max_gaia_sources"
v-model.number="max_sources"
type="number"
step="10"
:rules="[() => max_gaia_sources!=='' || 'This field is required']"
:rules="[
() => max_sources!=='' || 'This field is required',
max_sources => max_sources > 0 || 'Max sources must be a postive integer.',
]"
label="Max sources"
hint="Maximum number of sources."
persistent-hint
Expand Down
64 changes: 58 additions & 6 deletions jdaviz/configs/imviz/tests/test_catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@
from astropy.io import fits
from astropy.nddata import NDData
from astropy.coordinates import SkyCoord
from astropy.table import QTable
from astropy.table import Table, QTable


@pytest.mark.remote_data
class TestCatalogs:

# testing that the plugin search does not crash when no data/image is provided
def test_plugin_no_image(self, imviz_helper):
catalogs_plugin = imviz_helper.plugins["Catalog Search"]._obj
Expand Down Expand Up @@ -86,12 +85,27 @@ def test_plugin_image_with_result(self, imviz_helper, tmp_path):

catalogs_plugin = imviz_helper.plugins["Catalog Search"]._obj
catalogs_plugin.plugin_opened = True

# test SDSS catalog
catalogs_plugin.catalog.selected = 'SDSS'

# testing that SDSS catalog respects the maximum sources set
catalogs_plugin.max_sources = 100
catalogs_plugin.search(error_on_fail=True)

assert catalogs_plugin.results_available
assert catalogs_plugin.number_of_results == catalogs_plugin.max_sources

# reset max_sources to it's default value
catalogs_plugin.max_sources = 1000
# This basically calls the following under the hood:
# skycoord_center = SkyCoord(6.62754354, 1.54466139, unit="deg")
# zoom_radius = r_max = 3 * u.arcmin
# query_region_result = SDSS.query_region(skycoord_center, radius=zoom_radius, ...)
catalogs_plugin.search(error_on_fail=True)

assert catalogs_plugin.catalog.selected == 'SDSS'

# number of results should be > 500 or so
# Answer was determined by running the search with the image in the notebook.
assert catalogs_plugin.results_available
Expand All @@ -110,6 +124,9 @@ def test_plugin_image_with_result(self, imviz_helper, tmp_path):
tmp_file = tmp_path / 'test.ecsv'
qtable.write(tmp_file, overwrite=True)

# reset max_sources to it's default value
catalogs_plugin.max_sources = 1000

catalogs_plugin.from_file = str(tmp_file)
# setting filename from API will automatically set catalog to 'From File...'
assert catalogs_plugin.catalog.selected == 'From File...'
Expand All @@ -120,6 +137,21 @@ def test_plugin_image_with_result(self, imviz_helper, tmp_path):
catalogs_plugin.table.selected_rows = catalogs_plugin.table.items[0:2]
assert len(catalogs_plugin.table.selected_rows) == 2

# test Gaia catalog
catalogs_plugin.catalog.selected = 'Gaia'

assert catalogs_plugin.catalog.selected == 'Gaia'

# astroquery.gaia query has the Gaia.ROW_LIMIT parameter that limits the number of rows
# returned. Test to verify that this query functionality is maintained by the package.
# Note: astroquery.sdss does not have this parameter.
catalogs_plugin.max_sources = 10
with pytest.warns(ResourceWarning):
catalogs_plugin.search(error_on_fail=True)

assert catalogs_plugin.results_available
assert catalogs_plugin.number_of_results == catalogs_plugin.max_sources

assert imviz_helper.viewers['imviz-0']._obj.state.x_min == -0.5
assert imviz_helper.viewers['imviz-0']._obj.state.x_max == 2047.5
assert imviz_helper.viewers['imviz-0']._obj.state.y_min == -0.5
Expand Down Expand Up @@ -182,18 +214,38 @@ def test_offline_ecsv_catalog(imviz_helper, image_2d_wcs, tmp_path):
assert catalogs_plugin.number_of_results == n_entries
assert len(imviz_helper.app.data_collection) == 2 # image + markers

catalogs_plugin.table.selected_rows = [catalogs_plugin.table.items[0]]
assert len(catalogs_plugin.table.selected_rows) == 1

# test to ensure sources searched for respect the maximum sources traitlet
catalogs_plugin.max_sources = 1
catalogs_plugin.search(error_on_fail=True)
assert catalogs_plugin.number_of_results == catalogs_plugin.max_sources

catalogs_plugin.clear_table()

# test single source edge case and docs recommended input file type
sky_coord = SkyCoord(ra=337.5202807, dec=-20.83305528, unit='deg')
tbl = Table({'sky_centroid': [sky_coord], 'label': ['Source_1']})
tbl_file = str(tmp_path / 'sky_centroid1.ecsv')
tbl.write(tbl_file, overwrite=True)
n_entries = len(tbl)

catalogs_plugin.from_file = tbl_file
out_tbl = catalogs_plugin.search()
assert len([out_tbl]) == n_entries
assert catalogs_plugin.number_of_results == n_entries
assert len(imviz_helper.app.data_collection) == 2 # image + markers

catalogs_plugin.clear()

assert not catalogs_plugin.results_available
assert len(imviz_helper.app.data_collection) == 2 # markers still there, just hidden

catalogs_plugin.clear(hide_only=False)
assert not catalogs_plugin.results_available
assert len(imviz_helper.app.data_collection) == 1 # markers gone for good

catalogs_plugin.table.selected_rows = [catalogs_plugin.table.items[0]]

assert len(catalogs_plugin.table.selected_rows) == 1

assert imviz_helper.viewers['imviz-0']._obj.state.x_min == -0.5
assert imviz_helper.viewers['imviz-0']._obj.state.x_max == 9.5
assert imviz_helper.viewers['imviz-0']._obj.state.y_min == -0.5
Expand Down
Loading