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

MWS BHB VAC Review (DR1) #23

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

MWS BHB VAC Review (DR1) #23

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

Comments

@dylanagreen
Copy link

dylanagreen commented Dec 30, 2024

Contact Person: Amanda Byström
1 catalog file (fits), 1 README

Initial Checks:

  • Includes README
  • Columns in ALLCAPS
  • Extension names in ALLCAPS
  • Files include units (see below)

Initial Notes:

  • Please add a detailed data model to the README, including column names, descriptions and units where applicable. If some of the columns/HDUs are documented elsewhere, please include a link to where someone could find that documentation.
  • Please also add a contact person (similar to a "corresponding author" on a paper) to the README
  • 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
@dylanagreen dylanagreen self-assigned this Dec 30, 2024
@dylanagreen dylanagreen added the vac review Issues pertaining to VAC reviews label Jan 17, 2025
@dylanagreen
Copy link
Author

Re-review after updated version 2/13:

  • The five columns that were added, per the README, do not have their respective units in the actual catalog file, could you please add them to those columns?
  • The GAIA and other HDUs seem to have lost their units, can we please readd those as well?
  • For some reason fitsverify is throwing up errors on the checksum/datasum, after adding the units you will probably have to regenerate the checksums.

We also 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.0/). Do you have a preferred stubname? Otherwise I suggest a variation of mws-bhb

@dylanagreen
Copy link
Author

@weaverba137 Let's go ahead and move this to final review when you're ready. Note that there are two files in the top level directory iron_bhb_240830.fits and MWS_BHB.fits, the former is an earlier version of the catalog, and only the latter file is the actual catalog, so we can ignore the former. We will use stubname mws-bhb and version v1.0.

@weaverba137
Copy link
Member

Will do Tuesday morning.

@weaverba137
Copy link
Member

I've copied the files to the staging area, $CFS/desi/vac/dr1/mws-bhb/v1.0. The original README.md file is retained as README.md.bak. There are no issues with MWS_BHB.fits that I can see.

@dylanagreen
Copy link
Author

@weaverba137 Amanda has signed off on the staged files, and we've merged the README, so let's move this to final location and I'll close this off. Thanks!

@weaverba137
Copy link
Member

The files are in place, close when ready.

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

2 participants