Metadata Viewer: API to return dict, not list (#3292)

* Metadata Viewer: API to return dict, not list * Nested dict, deprecate instead of remove * Fix typo Co-authored-by: Kyle Conroy <[email protected]> --------- Co-authored-by: Kyle Conroy <[email protected]>
spacetelescope · Nov 18, 2024 · 896d4d9 · 896d4d9
1 parent 7e5ddfa
commit 896d4d9
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 11 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -29,6 +29,10 @@ API Changes
 
 - Renamed the ``Subset Tools`` plugin to ``Subsets`` which now exposes the ``subset``, ``combination_mode``, ``get_center``, and ``set_center`` in the user API. [#3293]
 
+- Metadata plugin: ``metadata_plugin.metadata`` API has been deprecated; use
+  ``metadata_plugin.meta`` instead, which will return a Python dictionary instead of
+  list of tuples. [#3292]
+
 Cubeviz
 ^^^^^^^
 

diff --git a/jdaviz/configs/default/plugins/metadata_viewer/metadata_viewer.py b/jdaviz/configs/default/plugins/metadata_viewer/metadata_viewer.py
@@ -23,7 +23,7 @@ class MetadataViewer(PluginTemplateMixin, DatasetSelectMixin):
       Dataset to expose the metadata.
     * :attr:`show_primary`:
       Whether to show MEF primary header metadata instead.
-    * :attr:`metadata`:
+    * :attr:`meta`:
       Read-only metadata. If the data is loaded from a multi-extension FITS file,
       this can be the extension header or the primary header, depending on
       ``show_primary`` setting.
@@ -47,7 +47,8 @@ def __init__(self, *args, **kwargs):
 
     @property
     def user_api(self):
-        return PluginUserApi(self, expose=('dataset', 'show_primary'), readonly=('metadata',))
+        return PluginUserApi(self, expose=('dataset', 'show_primary'),
+                             readonly=('metadata', 'meta'), deprecated=('metadata', ))
 
     def reset(self):
         self.has_metadata = False
@@ -100,8 +101,10 @@ def find_public_metadata(self, meta, primary_only=False):
 
         d = flatten_nested_dict(meta)
         # Some FITS keywords cause "# ipykernel cannot clean for JSON" messages.
-        # Also, we want to hide internal metadata that starts with underscore.
-        badkeys = ['COMMENT', 'HISTORY', ''] + [k for k in d if k.startswith('_')]
+        # Also, we want to hide internal metadata that starts with underscore
+        # and some "original_" entries from specutils.
+        badkeys = (['COMMENT', 'HISTORY', ''] + [k for k in d if k.startswith('_')]
+                   + [k for k in d if k.startswith('original_')])
         for badkey in badkeys:
             if badkey in d:
                 del d[badkey]
@@ -130,6 +133,11 @@ def get_comment(key):
         else:
             self.reset()
 
+    @property
+    def meta(self):
+        return dict((key, {"value": val, "description": cmt})
+                    for key, val, cmt in self.metadata)
+
 
 # TODO: If this generalized in stdatamodels in the future, replace with native function.
 #       See https://github.com/spacetelescope/stdatamodels/issues/131

diff --git a/jdaviz/configs/default/plugins/metadata_viewer/tests/test_metadata_viewer.py b/jdaviz/configs/default/plugins/metadata_viewer/tests/test_metadata_viewer.py
@@ -69,16 +69,23 @@ def test_view_dict(imviz_helper):
     assert mv.show_primary  # Make sure it sticks if possible
     assert mv.has_comments
     assert mv.metadata == [('APERTURE', '#TODO', 'Aperture'),
-                           ('BITPIX', '8', 'array data type'), ('EXTEND', 'True', ''),
+                           ('BITPIX', '8', 'array data type'),
+                           ('EXTEND', 'True', ''),
                            ('NAXIS', '0', 'number of array dimensions'),
                            ('SIMPLE', 'True', 'conforms to FITS standard')]
+    assert mv.meta == {'APERTURE': {"description": 'Aperture', "value": '#TODO'},
+                       'BITPIX': {"description": 'array data type', "value": '8'},
+                       'EXTEND': {"description": '', "value": 'True'},
+                       'NAXIS': {"description": 'number of array dimensions', "value": '0'},
+                       'SIMPLE': {"description": 'conforms to FITS standard', "value": 'True'}}
 
     mv.dataset_selected = 'no_meta'
     assert not mv.has_primary
     assert not mv.show_primary
     assert not mv.has_comments
     assert not mv.has_metadata
     assert mv.metadata == []
+    assert mv.meta == {}
 
 
 def test_view_invalid(imviz_helper):
@@ -94,3 +101,4 @@ def test_view_invalid(imviz_helper):
     assert not mv.has_comments
     assert not mv.has_metadata
     assert mv.metadata == []
+    assert mv.meta == {}
diff --git a/jdaviz/core/user_api.py b/jdaviz/core/user_api.py
@@ -4,20 +4,21 @@
 __all__ = ['UserApiWrapper', 'PluginUserApi', 'ViewerUserApi']
 
 _internal_attrs = ('_obj', '_expose', '_items', '_readonly', '_exclude_from_dict',
-                   '__doc__', '_deprecation_msg')
+                   '__doc__', '_deprecation_msg', '_deprecated')
 
 
 class UserApiWrapper:
     """
     This is an API wrapper around an internal object.  For a full list of attributes/methods,
     call dir(object).
     """
-    def __init__(self, obj, expose=[], readonly=[], exclude_from_dict=[]):
+    def __init__(self, obj, expose=[], readonly=[], exclude_from_dict=[], deprecated=[]):
         self._obj = obj
         self._expose = list(expose) + list(readonly)
         self._readonly = readonly
         self._exclude_from_dict = exclude_from_dict
         self._deprecation_msg = None
+        self._deprecated = deprecated
         if obj.__doc__ is not None:
             self.__doc__ = self.__doc__ + "\n\n\n" + obj.__doc__
 
@@ -34,6 +35,9 @@ def __getattr__(self, attr):
         if attr in _internal_attrs or attr not in self._expose:
             return super().__getattribute__(attr)
 
+        if attr in self._deprecated:
+            logging.warning("DeprecationWarning: %s is deprecated" % attr)
+
         exp_obj = getattr(self._obj, attr)
         return getattr(exp_obj, 'user_api', exp_obj)
 
@@ -127,13 +131,13 @@ class PluginUserApi(UserApiWrapper):
     For example::
       help(plugin_object.show)
     """
-    def __init__(self, plugin, expose=[], readonly=[], excl_from_dict=[]):
+    def __init__(self, plugin, expose=[], readonly=[], excl_from_dict=[], deprecated=[]):
         expose = list(set(list(expose) + ['open_in_tray', 'close_in_tray',
                                           'show']))
         if plugin.uses_active_status:
             expose += ['keep_active', 'as_active']
         self._deprecation_msg = None
-        super().__init__(plugin, expose, readonly, excl_from_dict)
+        super().__init__(plugin, expose, readonly, excl_from_dict, deprecated)
 
     def __repr__(self):
         if self._deprecation_msg:
@@ -151,9 +155,9 @@ class ViewerUserApi(UserApiWrapper):
     For example::
       help(viewer_object.show)
     """
-    def __init__(self, viewer, expose=[], readonly=[], excl_from_dict=[]):
+    def __init__(self, viewer, expose=[], readonly=[], excl_from_dict=[], deprecated=[]):
         expose = list(set(list(expose) + []))
-        super().__init__(viewer, expose, readonly, excl_from_dict)
+        super().__init__(viewer, expose, readonly, excl_from_dict, deprecated)
 
     def __repr__(self):
         return f'<{self._obj.reference} API>'