Metadata

[armengau]

• Major change:
  - Restructure "metadata" handling ie. code around viewer_cds.cds_targetinfo, now renamed viewer_cds.cds_metadata. Now much more metadata can be included, and are handled in a hopefully clean way.
  - Associated DataTables are handled in viewer_widget.add_metadata_tables : 3 or 4 DataTables are displayed, instead of one. Metadata to be displayed in the most visible table can be chosen with the option top_metadata=...
  - This solves several github issues.
• Other minor changes:
  - Bug fixes in archetypes.
  - Plot display: overlap_band; O-II zoom region; legend (legend outside plot: prototype, deactivated by default, before more feedback by VI people)
  - Make specutils dependency optional, with try..except on import.
  - Update "major" emission lines.
  - Renamed internal variables vi_class* to vi_quality*

[coveralls]

Coverage increased (+0.01%) to 1.461% when pulling 3c2c6b4 on metadata into 0592517 on master.

[weaverba137]

I will test as soon as I can, but I am very busy the next few days.

[weaverba137]

At first pass, it looks like the metadata processor is looking for header items that will definitely not be present in SDSS data:

---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-8-c5d782858dc8> in <module>
      2             zcatalog=sdss_z,
      3             model=(sdss.spectral_axis.value, sdss.meta['model'].to(u.Unit('1e-17 erg/(cm**2 s Angstrom)'))),
----> 4             notebook=True, title=os.path.basename('SDSS Plate %d' % plate), model_from_zcat=False, with_coaddcam=False, mask_type='PRIMTARGET', with_thumb_tab=False, with_vi_widgets=False)

~/Documents/Code/git/desihub/prospect/py/prospect/viewer/__init__.py in plotspectra(spectra, zcatalog, redrock_cat, notebook, html_dir, title, with_imaging, with_noise, with_thumb_tab, with_vi_widgets, top_metadata, vi_countdown, with_thumb_only_page, with_coaddcam, mask_type, model_from_zcat, model, num_approx_fits, with_full_2ndfit, template_dir, archetype_fit, archetypes_dir)
    365         template_dicts = None
    366 
--> 367     viewer_cds.load_metadata(spectra, mask_type=mask_type, zcatalog=zcatalog, survey=survey)
    368 
    369     #-------------------------

~/Documents/Code/git/desihub/prospect/py/prospect/viewer/cds.py in load_metadata(self, spectra, mask_type, zcatalog, survey)
    261             for xx,yy in spectra.meta.items() :
    262                 if yy=="desispec" : desispec_specversion = spectra.meta[xx.replace('NAM','VER')]
--> 263         self.cds_metadata.add([desispec_specversion for i in range(nspec)], name='spec_version')
    264         redrock_version = ["-1" for i in range(nspec)]
    265         if zcatalog is not None:

~/Documents/Code/git/desihub/prospect/py/prospect/viewer/cds.py in <listcomp>(.0)
    261             for xx,yy in spectra.meta.items() :
    262                 if yy=="desispec" : desispec_specversion = spectra.meta[xx.replace('NAM','VER')]
--> 263         self.cds_metadata.add([desispec_specversion for i in range(nspec)], name='spec_version')
    264         redrock_version = ["-1" for i in range(nspec)]
    265         if zcatalog is not None:

NameError: free variable 'desispec_specversion' referenced before assignment in enclosing scope

In addition, line 263 in this snippet:

    261             for xx,yy in spectra.meta.items() :
    262                 if yy=="desispec" : desispec_specversion = spectra.meta[xx.replace('NAM','VER')]
--> 263         self.cds_metadata.add([desispec_specversion for i in range(nspec)], name='spec_version')

makes the assumption that the loop in lines 261-262 is always successful. This is not guaranteed even for DESI data, especially as we churn through constant commissioning & SV
code updates.

[armengau]

    261             for xx,yy in spectra.meta.items() :
    262                 if yy=="desispec" : desispec_specversion = spectra.meta[xx.replace('NAM','VER')]
--> 263         self.cds_metadata.add([desispec_specversion for i in range(nspec)], name='spec_version')

This was a bug, two variable names for one (desispec_version vs desispec_specversion). I didnt hit this since indeed I didn't test on SDSS. I replaced both previous names by spec_version.

[weaverba137]

I'm getting a JavaScript error after loading prospect with SDSS data. It appears that some object in this line from update_plot.js is not actually defined:

            shortcds_table_z.data['SPECTYPE'] = [ metadata.data['SPECTYPE'][ifiber] ];

Among other things, this prevents advancing from one spectrum to the next.

[weaverba137]

I noted a serious error in my previous comment on the main PR discussion page. I have a few more minor comments in this review.

[armengau]

I'm getting a JavaScript error after loading prospect with SDSS data. It appears that some object in this line from update_plot.js is not actually defined:

            shortcds_table_z.data['SPECTYPE'] = [ metadata.data['SPECTYPE'][ifiber] ];

Among other things, this prevents advancing from one spectrum to the next.

You're right, zcatalog-related metadata were not properly implemented in the case of SDSS.
That's done (hopefully) now.

This time I tested the viewer on SDSS data, based on the notebook in doc/nb/Prospect_specutils.ipynb. From what I can see it works well.

[weaverba137]

Thank you @armengau, I will test later today.

[weaverba137]

I found no problems with my own tests this time. Merge when ready.