-
Notifications
You must be signed in to change notification settings - Fork 441
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 Pairtools sort #5910
Add Pairtools sort #5910
Conversation
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Fixed errors, added more options and tests for the tool
Restructured the folder structure
Reduced the size of test-data
…'" wherever we can.
tools/pairtools/sort.xml
Outdated
--nproc-out \${GALAXY_SLOTS:-4} | ||
]]></command> | ||
<inputs> | ||
<param name="pairs_path" type="data" format="pairs" label="Input pairs file" help="Input .pairs/.pairsam file"/> |
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.
pairs is not a valid Galaxy format
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.
:(
in the parse xml we have this line and it worked...
<data name="output_parsed_pairs" format="pairs"
I can change it to text,tabular. Also pairsam?
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 just double checked, .pairs and .pairsam both are tabular format data as per https://pairtools.readthedocs.io/en/latest/formats.html#pairs
If we all agree, we can stick to "format=tabular"
? If not, we are also happy to include .pairs and .pairsam format to Galaxy.
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.
Ok, I'll update all to tabular
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.
Hi @diegomics,
A small suggestion inline.
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
Co-authored-by: Saim Momin <[email protected]>
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.
We need to decide if we add the PAIR datatype to Galaxy or if we go for tabular for the time being.
tools/pairtools/sort.xml
Outdated
<data name="output_sorted_pairs" format="pairs,pairsam" label="${tool.name} on ${on_string}: .pairsam"/> | ||
</data> |
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.
It can only be one format.
<data name="output_sorted_pairs" format="pairs,pairsam" label="${tool.name} on ${on_string}: .pairsam"/> | |
</data> | |
<data name="output_sorted_pairs" format="pairs,pairsam" label="${tool.name} on ${on_string}: .pairsam"/> |
This seems to be the format, correct? https://github.com/4dn-dcic/pairix/blob/master/pairs_format_specification.md
We should create a Galaxy datatype for it. @SaimMomin12 maybe we can talk about that tomorrow?
Co-authored-by: Björn Grüning <[email protected]>
Co-authored-by: Björn Grüning <[email protected]>
Resolved: #5917 |
Nice! Yes... both generating part of the files for the test dataset messed up the diff. I'm closing this PR now that #5917 is done |
FOR CONTRIBUTOR: