astro-datalab / notebooks-latest

Default set of Data Lab notebooks, by DL team and contributed by users
BSD 3-Clause "New" or "Revised" License
57 stars 48 forks source link

Notebook for SPARCL + Jdaviz #205

Closed weaverba137 closed 5 months ago

weaverba137 commented 1 year ago

This PR adds a how-to for displaying SPARCL spectra in Jdaviz (specifically Specivz). This notebook was initially developed at .Astronomy 12.

jacquesalice commented 1 year ago

Hi @weaverba137 I've gone through and added section descriptions and other small formatting edits. Jdaviz is available on gp02 if you would like to review/test it on there. @rnikutta and @galaxyumi , can yall take a look at this notebook as well?

galaxyumi commented 1 year ago

@weaverba137 and @jacquesalice, here are more comments:

  1. The last two commente-out lines in the cell where you define the function on_selected can be deleted if they are no longer in use.
  2. The last cell for making plot could have more description in the markdown. For example, featuring a drop down bar where one can select a target to display its spectrum with the best-fit model.
  3. The y-axis label is not following the scientific unit style conventions, e.g.,[erg s^{-1} cm^{-2} A^{-1}].
  4. The x-axis ticks are too busy to read. Maybe changing to nm? I am getting a long warning box complaining about the Angstrom anyway.
  5. It would be great if the plot's legend could be more informative. Currently, they are random(?) two alphabets in red and black colors for model and data.
  6. Please take care of this long warning if possible. Otherwise, please leave a comment about the warnings in the markdown not to scare the users. Screen Shot 2023-10-16 at 4 50 06 PM
weaverba137 commented 1 year ago

From the comments above:

  1. Fixed.
  2. Fixed.
  3. Waiting on a fully viable notebook server to test. This may be an internal feature of Jdaviz that cannot be changed.
  4. Waiting on a fully viable notebook server to test. This may be an internal feature of Jdaviz that cannot be changed.
  5. The alphabetical indexes are an internal feature of Jdaviz. We cannot change that in this notebook.
  6. Waiting on a fully viable notebook server to fix.
weaverba137 commented 1 year ago

Updates:

@jacquesalice, I think this is ready to go, but it should be tested on gp02 when it becomes available again, and also tested on gp12 when Jdaviz is installed on that system.

weaverba137 commented 1 year ago

PS, I updated the HTML version on the last commit. Please double-check that it looks OK.

weaverba137 commented 10 months ago

I've retested this notebook with the new 'Python 3' kernel on gp02. Please review again. There are no major changes from last time, but it has been a while since we were able to test this.

stephjuneau commented 10 months ago

Hi @weaverba137 , @jacquesalice , @galaxyumi : checking on the status of this notebook. From Ben's last comment it was retested on gp02. What are the next steps after reviewing this version before it can be deployed to prod/gp12?

weaverba137 commented 10 months ago

I believe it requires the Python 3.10 kernel to be installed on the production server, so that might also be a question for @mjfitzpatrick.

weaverba137 commented 5 months ago

@jacquesalice, it should be possible to at least review this notebook on gp13 so that it is ready to merge if not yet fully merged.