Add compression option to export #114
Conversation
There was a problem hiding this comment.
LGTM! But I would recommend @irisdianauy to give the final approval as she reviewed #112
| "--mtx-compression", | ||
| "-G", | ||
| type=click.Choice(["zip", "gzip", "bz2", "zstd"]), | ||
| default=None, |
There was a problem hiding this comment.
should the default option be one of the compressed options?
There was a problem hiding this comment.
I think it's okay to have this set to None since the underlying function to generate the matrix accepts None (no compression) and will not return an error. But you're right that it might be a good idea feature-wise, if a compressed matrix is more commonly used directly downstream, e.g., direct downstream tools can accept compressed files (?).
There was a problem hiding this comment.
@pmb59 @irisdianauy Thanks for the feedback! As a general strategy we always try to add functionality in a way that doesn't change things by default, so we'll leave uncompressed as the option unless the user specifies otherwise. To do otherwise risks breaking the pipelines of anyone using these tools.
| "--mtx-compression", | ||
| "-G", | ||
| type=click.Choice(["zip", "gzip", "bz2", "zstd"]), | ||
| default=None, |
There was a problem hiding this comment.
I think it's okay to have this set to None since the underlying function to generate the matrix accepts None (no compression) and will not return an error. But you're right that it might be a good idea feature-wise, if a compressed matrix is more commonly used directly downstream, e.g., direct downstream tools can accept compressed files (?).
This PR makes the changes from #112 available via the command-line interface so we can actually leverage that direct compression in our Galaxy tools etc.