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
30 changes: 20 additions & 10 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 @@ -159,6 +159,8 @@
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]

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

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/imviz/plugins/catalogs/catalogs.py#L163

Added line #L163 was not covered by tests
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.

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 +184,12 @@
unit='deg')

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

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

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/imviz/plugins/catalogs/catalogs.py#L187

Added line #L187 was not covered by tests

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

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

View check run for this annotation

Codecov / codecov/patch

jdaviz/configs/imviz/plugins/catalogs/catalogs.py#L189-L190

Added lines #L189 - L190 were not covered by tests
columns=('source_id', 'ra', 'dec')
)
self.app._catalog_source_table = sources
skycoord_table = SkyCoord(sources['ra'], sources['dec'], unit='deg')

Expand All @@ -195,8 +198,10 @@
# 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
else:
skycoord_table = table['sky_centroid']
else:
self.results_available = False
self.number_of_results = 0
Expand Down Expand Up @@ -243,16 +248,21 @@
# NOTE: If performance becomes a problem, see
# https://docs.astropy.org/en/stable/table/index.html#performance-tips
if self.catalog_selected == '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
30 changes: 25 additions & 5 deletions jdaviz/configs/imviz/tests/test_catalogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
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
Expand Down Expand Up @@ -182,18 +182,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