Skip to content

[query] Support zstd compression in BGEN files#12576

Merged
danking merged 2 commits into
hail-is:mainfrom
tpoterba:bgen-zsstd
Jan 13, 2023
Merged

[query] Support zstd compression in BGEN files#12576
danking merged 2 commits into
hail-is:mainfrom
tpoterba:bgen-zsstd

Conversation

@tpoterba

@tpoterba tpoterba commented Jan 6, 2023

Copy link
Copy Markdown
Contributor

CHANGELOG: hl.import_bgen and hl.export_bgen now support compression with Zstd.

CHANGELOG: `hl.import_bgen` and `hl.export_bgen` now support compression with Zstd.

@chrisvittal chrisvittal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One small maintainability issue around the ids for compression.

assert(compressionCodec == "zlib" || compressionCodec == "zstd")
val writeHeader = exportType == ExportType.PARALLEL_HEADER_IN_SHARD
val partWriter = BGENPartitionWriter(tm, entriesFieldName, writeHeader)
val compressionInt = compressionCodec match {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any way to make this some kind of enum, or if that's too much, named constants?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

@danking danking merged commit eee0924 into hail-is:main Jan 13, 2023
danking pushed a commit to danking/hail that referenced this pull request Jan 30, 2023
* [query] Support zstd compression in BGEN files

CHANGELOG: `hl.import_bgen` and `hl.export_bgen` now support compression with Zstd.

* address comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants