Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feature/bruker data #275
Feature/bruker data #275
Changes from 16 commits
c4b17e4
0b55503
4e9a931
51a6e66
9cc93fa
e8752f6
b559b21
0793038
224e340
9e4c872
1a86393
92ee452
33b3579
6ae3565
4b58fba
288415a
ffff4c9
d9feb35
5e8fa66
bf084e2
786798f
ec37001
882b968
69ae560
266c121
4d94097
41f76c2
7ce33c4
454b911
85f3060
21ee38b
f4d8cbe
062e6ba
6c933e4
282b9aa
aa32722
0da9965
f755690
83904d9
88ef3f3
484a19c
bbd8f63
50b1205
0b11023
bcc5295
07a6ed2
b72e000
54b685b
f1b0bf7
503538d
ce2670e
b20d91c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
TODO add branch here with a plain .d and mix them (and add the exception to TDF2MZML)
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.
@jspaezp wouldn't be more interesting to have
.d
gzip rather than.tar
. Im asking because I have seen most of the compressed files using gzip instead of tar.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.
I could implement either, We tested locally and since most of the data inside the file is already compressed, gzip offered very little benefit (<10% compression) whilst dramatically increasing the time it took to generate (4x longer if i recall correctly). So in our use case at least it was not worth having the compression.
I could definitely add something like this to the extraction step:
https://gist.github.com/hightemp/5071909#file-bash-aliases-L32-L60
and correspondingly I would just have the branch be something like
I will add this to the options (I think we should enforce having the .d though. I would be very hard to track file properties if we attempt to allow "myfile.tar.gz")
Will add this in the next commit
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.
@jspaezp interesting the information about why do you use
.tar
. Would be great to support this approach for those ones using other formats.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.
Note to self:
quantms/lib/WorkflowQuantms.groovy
Lines 72 to 74 in 7363d90
since the extension is checked here, a more verbose branching is needed, I think that supporting tar/tar.gz/tar.bz/zip should be enough for now.
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.
added here: 85f3060
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.
@ypriverol
In a semi-related manner. Do you have any suggestion on public data? If not I think we could upload something.
I was thinking on maybe using https://www.ebi.ac.uk/pride/archive/projects/PXD034128 but its phospho, so it might take A WHILE to search. AND it seems like most people are uploading the data as a single .zip with all the files (and I am not sure if we want to support that or if there is a way to stage the files more efficiently).
LMK what you think