-
Notifications
You must be signed in to change notification settings - Fork 77
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
add default compression to write_dataframe function to compress dl2 #1165
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1165 +/- ##
=======================================
Coverage 73.97% 73.97%
=======================================
Files 124 124
Lines 12647 12647
=======================================
Hits 9356 9356
Misses 3291 3291
☔ View full report in Codecov by Sentry. |
Well spotted... indeed DL2 are really bulkier than the corresponding DL1b, funny we have not noticed. |
Hi @moralejo I just did the test and here are the results.
|
@vuillaut Did you test that locally or on the cluster? Because I'd suspect that write and read speed actually would go up when writing much less data to the slowish network file system on the cluster. |
…into compress_dl2
Indeed, I tested locally on my laptop, let me run this on the cluster. |
I updated the table in my previous comment with numbers from the test at la palma |
+1 to setting the compression level to 1 by default |
I have set default
(I don't think the read time are too relevant, they probably fluctuate too much with cluster usage) I think this is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks. I left some comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
…aframes. update complevel=1 for all"
Hey @morcuended
I'd say the conclusion is the same as for parameters, so I did change the complevel to 1 as default for everyone. I also simplified the Could you review again, please? |
Thanks Daniel! |
Apologies for missed review and thanks for this @vuillaut ! |
fixes #1163