Make metadata creation optional #605
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #605 +/- ##
==========================================
- Coverage 92.92% 92.38% -0.54%
==========================================
Files 49 49
Lines 2261 2286 +25
==========================================
+ Hits 2101 2112 +11
- Misses 160 174 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Click here to view all benchmarks. |
delucchi-cmu
left a comment
delucchi-cmu
left a comment
There was a problem hiding this comment.
Code changes look great - a few issues with the docstring rendering: https://hats--605.org.readthedocs.build/en/605/autoapi/hats/io/parquet_metadata/index.html#hats.io.parquet_metadata.write_parquet_metadata
src/hats/io/parquet_metadata.py
Outdated
| For more information on the general parquet metadata files, and why we write them, see | ||
| Creates files (relative to the catalog root): | ||
| data_thumbnail.parquet (only if create_thumbnail=True) |
There was a problem hiding this comment.
This is written to dataset/data_thumbnail.parquet
There was a problem hiding this comment.
Looking at this diagram in the IVOA note, it seems to be at the root level - have we changed the behavior since?
src/hats/io/parquet_metadata.py
Outdated
| """Write Parquet dataset-level metadata files (and optional thumbnail) for a catalog. | ||
| For more information on the general parquet metadata files, and why we write them, see | ||
| Creates files (relative to the catalog root): |
There was a problem hiding this comment.
This table isn't rendered well by sphinx, and the connection to the below paragraphs is awkward. How about:
* thumbnail
* only if create_thumbnail=True
* <purpose and contents>
* _common_metadata
* always written
* <purpose and contents>
* etc...
There was a problem hiding this comment.
Updated based on our conversation earlier today, see rendered docs here: https://hats--605.org.readthedocs.build/en/605/autoapi/hats/io/parquet_metadata/index.html
I mostly copied the descriptions from the IVOA note, and ofc totally open to revisions to writing/formatting/etc
src/hats/io/parquet_metadata.py
Outdated
| The `data_thumbnail.parquet` file contains one row from each data partition, | ||
| up to a maximum of `thumbnail_threshold` rows. | ||
| References |
There was a problem hiding this comment.
Sphinx doesn't know about References. Maybe Notes? It's also not happy about the bullets.
Co-authored-by: Melissa DeLucchi <[email protected]>
Updated references section to notes in docstring.
…ing soon to a bookstore near you
Closes (with other PR) astronomy-commons/hats-import#628
The PR-pair adds a
create_metadataflag to hats-import to make the creation of the /dataset/_metadata` parquet file optional (but created by default.This PR adds a
create_metadataflag towrite_parquet_metadataspecifically, which is the method called anytime we wish to create the_metadatafile from hats-import.