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

HETDEX VAC Review (DR1) #24

Closed
4 tasks done
dylanagreen opened this issue Dec 30, 2024 · 8 comments
Closed
4 tasks done

HETDEX VAC Review (DR1) #24

dylanagreen opened this issue Dec 30, 2024 · 8 comments
Assignees
Labels
vac review Issues pertaining to VAC reviews

Comments

@dylanagreen
Copy link

dylanagreen commented Dec 30, 2024

Contact Person: Martin Landriau
1 catalog file (fits), 1 README, 1 jupyter notebook

Initial Checks:

  • Includes README
  • Columns in ALLCAPS
  • Extension names in ALLCAPS
  • Files include units

Initial Notes:

  • For consistency and ease of use we usually require that all columns be in ALL CAPS, can you please update the column names in the INFO header to be ALLCAPS?
  • Please also add a contact person (similar to a "corresponding author" on a paper) to the README
  • Just to be sure, since this VAC includes HETDEX spectra, are the spectra already publicly accessible or do we need to obtain permission to release them?
  • The jupyter notebook refers to V1.4 of the catalog file but the directory contains V1.5
  • Can you add the version to the directory path? (e.g. VAC_DR1/v1.5/)
  • The Jupyter notebook looks good, in order to publicly release we request that you please make a static html version that can be displayed online (e.g. the AGN physical properties VAC from EDR has one hosted as part of their VAC here: https://data.desi.lbl.gov/public/edr/vac/edr/cigale/fuji/v1.4/CigaleExampleFuji.html). You can do this directly from jupyterlab, save and export as html.
@dylanagreen dylanagreen self-assigned this Dec 30, 2024
@dylanagreen
Copy link
Author

@mlandriau reached out privately and indicated this was ready for a re-review. Thanks for making these changes! I have just a few more notes here before final review:

1

The table itself is looking pretty good! I'm not seeing anything in my checks that gives me pause. Thanks for making an html version of the notebook too, I don't see anything there that needs to be updated either.

2

I would like to encourage you to include a checksum in your fits file, you can do this easily using astropy: https://docs.astropy.org/en/stable/io/fits/api/tables.html#astropy.io.fits.BinTableHDU.add_checksum and https://docs.astropy.org/en/stable/io/fits/api/images.html#astropy.io.fits.ImageHDU.add_checksum

3

Can you add a contact person to the README (akin to a "corresponding author" for a paper)? In addition, to make sure we're not accidentally releasing any proprietary data that DESI does not own, can you also include a short description of where the HETDEX spectra are drawn from (i.e. a link to a public data release or public access point)?

4

We need to decide on a stubname. The stubname will be used for the README file as well as the on-disk location for the vac (e.g. /dr1/stubname/v1.6/). Do you have a preferred stubname? Otherwise I suggest a variation of hetdex or hetdex-desi

Once we square these away I think we're looking good for final review.

@dylanagreen dylanagreen added the vac review Issues pertaining to VAC reviews label Jan 17, 2025
@mlandriau
Copy link

@dylanagreen Thanks for the review!

  1. I've added checksums to all hdus. I've kept the file version number the same (1.6), though: let me know if I should amend that.

  2. I've amended the README.md to include myself and another person as contacts, and I've added the URL for the HETDEX public data from which the data in the VAC comes from.

  3. hetdex is perfect.

@dylanagreen
Copy link
Author

dylanagreen commented Feb 3, 2025

@weaverba137 let's move this VAC to final review, with the stubname hetdex, thanks Martin for all your work on this!

@weaverba137
Copy link
Member

OK, I see a FITS file, a Jupyter notebook and an HTML file. Can we confirm that the HTML file a rendering of the Jupyter notebook? In the meantime, I have copied the files into the VAC staging area.

@mlandriau
Copy link

@weaverba137 Yes, the HTML file is just a rendering of the notebook.

@dylanagreen
Copy link
Author

@weaverba137 Since we've now merged and finalized the README for this VAC, if there's nothing else to do on the actual VAC itself let's move it to the final location and then I'll close off this issue. Thanks!

@weaverba137
Copy link
Member

The VAC is now in the final public area. Close when ready.

@dylanagreen
Copy link
Author

Thanks Ben and Martin for all your work on this, this VAC is officially finalized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vac review Issues pertaining to VAC reviews
Projects
None yet
Development

No branches or pull requests

3 participants