Apply suggestions from code review

Co-authored-by: P. L. Lim <[email protected]>

CHANGES.rst

@@ -12,7 +12,7 @@ Cubeviz
 Imviz
 ^^^^^
 
-- Catalogs plugin now supports loading a JWST catalog from a local ecsv file. [#1707]
+- Catalogs plugin now supports loading a JWST catalog from a local ECSV file. [#1707]
 
 Mosviz
 ^^^^^^

docs/conf.py

@@ -260,7 +260,6 @@
     nitpick_ignore.append((dtype, target))
 
 # Extra intersphinx in addition to what is already in sphinx-astropy
-intersphinx_mapping['astropy'] = ('https://docs.astropy.org/en/stable/', None)
 intersphinx_mapping['glue'] = ('http://docs.glueviz.org/en/stable/', None)
 intersphinx_mapping['glue_jupyter'] = ('https://glue-jupyter.readthedocs.io/en/stable/', None)
 intersphinx_mapping['regions'] = ('https://astropy-regions.readthedocs.io/en/stable/', None)

docs/imviz/plugins.rst

@@ -223,7 +223,7 @@ catalog dropdown menu.
     This plugin is still under active development. As a result, the search only uses the SDSS DR17 catalog
     and works best when you only have a single image loaded in a viewer.
 
-To load a catalog from a supported `JWST ecsv catalog file <https://jwst-pipeline.readthedocs.io/en/latest/jwst/source_catalog/main.html#output-products>`_, choose "From File...".  
+To load a catalog from a supported `JWST ECSV catalog file <https://jwst-pipeline.readthedocs.io/en/latest/jwst/source_catalog/main.html#output-products>`_, choose "From File...".  
 The file must be able to be parsed by `astropy.table.Table.read` and contain a column labeled 'sky_centroid'.
 Clicking :guilabel:`SEARCH` will show markers for any entry within the filtered zoom window.
 

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

@@ -65,7 +65,7 @@ def _on_file_path_changed(self, event):
             self.from_file_message = 'Could not parse file with astropy.table.QTable.read'
             return
 
-        if 'sky_centroid' not in table.keys():
+        if 'sky_centroid' not in table.colnames:
             self.from_file_message = 'Table does not contain required sky_centroid column'
             return
 

jdaviz/configs/imviz/tests/test_catalogs.py

@@ -118,30 +118,31 @@ def test_plugin_image_with_result(self, imviz_helper, tmp_path):
         assert catalogs_plugin.results_available
         assert catalogs_plugin.number_of_results == 2473
 
-    def test_from_file_parsing(self, imviz_helper, tmp_path):
-        catalogs_plugin = imviz_helper.app.get_tray_item_from_name('imviz-catalogs')
-
-        # _on_file_path_changed is fired when changing the selection in the file dialog
-        catalogs_plugin._on_file_path_changed({'new': './invalid_path'})
-        assert catalogs_plugin.from_file_message == 'File path does not exist'
-
-        # observe('from_file') is fired when setting from_file from the API (or after clicking
-        # select in the file dialog)
-        with pytest.raises(ValueError, match='./invalid_path does not exist'):
-            catalogs_plugin.from_file = './invalid_path'
-
-        # setting to a blank string from the API resets the catalog selection to the
-        # default/first entry
-        catalogs_plugin.from_file = ''
-        assert catalogs_plugin.catalog.selected == catalogs_plugin.catalog.choices[0]
-
-        not_table_file = tmp_path / 'not_table.tst'
-        not_table_file.touch()
-        catalogs_plugin._on_file_path_changed({'new': not_table_file})
-        assert catalogs_plugin.from_file_message == 'Could not parse file with astropy.table.QTable.read'  # noqa
-
-        qtable = QTable({'not_sky_centroid': [1, 2, 3]})
-        not_valid_table = tmp_path / 'not_valid_table.ecsv'
-        qtable.write(not_valid_table, overwrite=True)
-        catalogs_plugin._on_file_path_changed({'new': not_valid_table})
-        assert catalogs_plugin.from_file_message == 'Table does not contain required sky_centroid column'  # noqa
+
+def test_from_file_parsing(imviz_helper, tmp_path):
+    catalogs_plugin = imviz_helper.app.get_tray_item_from_name('imviz-catalogs')
+
+    # _on_file_path_changed is fired when changing the selection in the file dialog
+    catalogs_plugin._on_file_path_changed({'new': './invalid_path'})
+    assert catalogs_plugin.from_file_message == 'File path does not exist'
+
+    # observe('from_file') is fired when setting from_file from the API (or after clicking
+    # select in the file dialog)
+    with pytest.raises(ValueError, match='./invalid_path does not exist'):
+        catalogs_plugin.from_file = './invalid_path'
+
+    # setting to a blank string from the API resets the catalog selection to the
+    # default/first entry
+    catalogs_plugin.from_file = ''
+    assert catalogs_plugin.catalog.selected == catalogs_plugin.catalog.choices[0]
+
+    not_table_file = tmp_path / 'not_table.tst'
+    not_table_file.touch()
+    catalogs_plugin._on_file_path_changed({'new': not_table_file})
+    assert catalogs_plugin.from_file_message == 'Could not parse file with astropy.table.QTable.read'  # noqa
+
+    qtable = QTable({'not_sky_centroid': [1, 2, 3]})
+    not_valid_table = tmp_path / 'not_valid_table.ecsv'
+    qtable.write(not_valid_table, overwrite=True)
+    catalogs_plugin._on_file_path_changed({'new': not_valid_table})
+    assert catalogs_plugin.from_file_message == 'Table does not contain required sky_centroid column'  # noqa