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

Add herschel_get_spec functionality to spectroscopy notebook #284

Merged
merged 5 commits into from
Jul 24, 2024

Conversation

jkrick
Copy link
Contributor

@jkrick jkrick commented Jun 13, 2024

This new module downloads and extracts "reduced" Herschel spectroscopy for a set of sources. It is completely functional, but not perfect. Description of imperfections (with short rants included) below:

  1. Some decisions had to be made as this is a complicated dataset with complicated sets of rules about when a final spectrum is generated and which fluxes of the many reported are the best to use for any given target.

    • only observations which produce a HPSSPEC directory are considered. There seems to be no way to ascertain this in the archive search, so the code downloads all tar files for all observations and then checks to see if this directory exists.

    • source resolution matters, ie., recommendations for which reduced spectrum to use are based on wether or not it is a point source or how extended it is. I have followed the advice in https://www.cosmos.esa.int/documents/12133/996891/Product+decision+trees to use the flux column which has the largest sum. It is beyond the scope of this notebook to calculate if the sources is extended in Herschel band or not.

    • The Herschel directory tree in the downloaded tar files is extensive, and there is no way to customize and only download the one file that I need, so there is a lot of code around tracking filenames and paths and searching paths for filenames.

    • The Herschel archive breaks the connection at some point (testing on Arp220 which has many observations > 50, will hit this error). I cannot for the life of me figure out how to catch the specific exceptions, and have spent too long trying to do this. So I know that blanket exceptions are not good practice, but they are the only way I know how to get this to work. I have opened an issue on this both here (fix open 'try except' call in herschel_get_spec function #282 ) and at astroquery (to try to fix this at the source).

    • I have written into the function an option to delete tar files after the spectrum is extracted.

  2. I re-numbered the sections to account for the fact that Herschel data is being accessed through the ESA archive and not IRSA as was originally suggested in the outline. The use of astroquery's ESA module made this module possible in an automated fashion from python, so it is not possible to switch to IRSA. IRSA only houses an API (which may even just access the ESA API???).

  3. It works with the plotting function before Andreas's newest PR Spectroscopy Notebook Updates #281 , but probably needs to be checked for compatibility with new function. I have opened an issue for this (Test plotting of Herschel data after PR #281 closes #283 ).

@jkrick jkrick added the use case: spectroscopy Spectroscopy use case label Jun 13, 2024
@jkrick jkrick self-assigned this Jun 13, 2024
@jkrick jkrick requested a review from afaisst June 26, 2024 21:32
@afaisst
Copy link
Contributor

afaisst commented Jul 24, 2024

Works. The only problem is that it takes ages (meaning hours) to get the Herschel data for only 5 sources.

Copy link
Contributor

@afaisst afaisst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!
I also checked that it works with the modified plotting code.

@jkrick jkrick merged commit d617cea into main Jul 24, 2024
3 checks passed
@jkrick jkrick deleted the add_herschel branch July 24, 2024 16:33
github-actions bot pushed a commit that referenced this pull request Jul 24, 2024
Add herschel_get_spec functionality to spectroscopy notebook d617cea
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
use case: spectroscopy Spectroscopy use case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants