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

DESI Strong Lensing Catalog VAC Review (DR1) #17

Closed
2 tasks done
dylanagreen opened this issue Dec 30, 2024 · 10 comments
Closed
2 tasks done

DESI Strong Lensing Catalog VAC Review (DR1) #17

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

Comments

@dylanagreen
Copy link

Contact Person: Christopher Storfer
1 catalog file (csv), 1 README, 1 additional file

Initial Checks:

  • Includes README
  • Columns in ALLCAPS

Initial Notes:

  • Column 0 (an index column?) has no name. Can you please add a name or otherwise remove the column?
  • When you get an opportunity, please flesh out the README with a full data model for the currently included columns, as well as a contact person to release publicly.
  • There's an included jupyter notebook file, but it appears to be empty. Is this intended? If not, can you please update the notebook with the intended detail? Otherwise let's delete this file.
  • There's a version number in the VAC file, but not in the file path (i.e. /SL-VAC/v0), can you please add one?
@dylanagreen dylanagreen changed the title DESi Strong Lensing Catalog VAC Review (DR1) DESI Strong Lensing Catalog VAC Review (DR1) Dec 30, 2024
@dylanagreen dylanagreen self-assigned this Dec 30, 2024
@dylanagreen dylanagreen added the vac review Issues pertaining to VAC reviews label Jan 17, 2025
@cjstorfer
Copy link

All comments have been addressed.

@dylanagreen
Copy link
Author

@weaverba137 Perlmutter is currently down, but when you have a chance (possibly when it goes up) let's move this to final review with stubname strong-lensing and version v1.0/.

@weaverba137
Copy link
Member

The strings in the CSV file are written out as, e.g.:

39627909152900068,b'main',b'bright',

I don't think this is standard for CSV files of any type. If someone thinks, "this is a CSV file, I can open it in Excel", they may have problems.

Is there a particular reason the format has to be CSV? Could ECSV or FITS be used instead? That would almost certainly take care of the formatting issue one way or another.

The description above mentions "1 additional file". What is this file?

@weaverba137
Copy link
Member

Also, the file still retains v0, even though we're calling this v1.0.

@dylanagreen
Copy link
Author

@weaverba137 I will talk to Chrstopher re: version and formatting. In regards to the additional file there used to be a jupyter notebook but in the latest version they deleted it so the VAC is just the catalog file and the README now.

@weaverba137
Copy link
Member

OK, thank you.

@dylanagreen
Copy link
Author

@weaverba137 Christopher has updated the file to be a fits table instead, with units and a checksum added to the file as well as renaming it to be v1. Which should resolve the lingering concerns.

@weaverba137
Copy link
Member

The FITS file looks fine to me. I've prepped the staging area for moving to the public area.

@dylanagreen
Copy link
Author

Now that we've merged the README, @weaverba137 you can move this to the final location and I'll close this issue off when ready. 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

3 participants