-
Notifications
You must be signed in to change notification settings - Fork 58
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
[WIP] ProteoRE 'filter kw and values' tool - Version 2021.06.01 #590
base: master
Are you sure you want to change the base?
[WIP] ProteoRE 'filter kw and values' tool - Version 2021.06.01 #590
Conversation
@combesf awesome! We will try to review asap! Keep them coming! |
<param name="inclusive" type="boolean" label="inclusive range ?" checked="false" truevalue="true" falsevalue="false" /> | ||
</repeat> | ||
<conditional name="sort"> | ||
<param name="sort_bool" type="boolean" label="Sort by column ?" checked="false" truevalue="true" falsevalue="false" /> |
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.
This should be a select by best practices.
</conditional> | ||
</inputs> | ||
<outputs> | ||
<data name="discarded_lines" format="tsv" label="Filtered_${input1.name} - discarded_lines" /> |
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 would prefer more standard labels: ${tool.name} on ${on_string}: discarded_lines
</inputs> | ||
<outputs> | ||
<data name="discarded_lines" format="tsv" label="Filtered_${input1.name} - discarded_lines" /> | ||
<data name="kept_lines" format="tsv" label="Filtered_${input1.name}" /> |
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.
${tool.name} on ${on_string}
<conditional name="k" > | ||
<param name="kw" type="select" label="Enter keywords" > | ||
<option value="text" selected="true">copy/paste</option> | ||
<option value="file">File containing keywords</option> |
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.
Can you add a test for keyword file
<inputs> | ||
<param type="data" name="input1" format="txt,tabular" label="Input file" /> | ||
<param name="header" type="boolean" checked="true" truevalue="true" falsevalue="false" label="Does file contain header?" /> | ||
<param name="operation" type="select" label="Operation" help="keep or discard word(s) or value(s) that match filters ?"> |
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.
The tool would become even more powerful if the keep/discard could be selected for each filter, or? Maybe leaving this parameter as default.
</param> | ||
<param name="value" type="float" value="" label="Value"></param> | ||
</repeat> | ||
<repeat name="values_range" title="Filter by range of numerical values"> |
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.
Can you add this to the test?
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.
What is the version in the PR title refering to? Version 2021.04.19.1
?
Do you intend to keep this https://github.com/vloux/ProteoRE/tree/master/tools/proteore_filter_keywords_values? Then I would prefer to make the proteore repo a proper python package released on pypi and bioconda which we could then use as requirement.
Hello Matthias, |
From ProteoRE team : as agreed after the Valentin's PR (see here)
#535
We now make the PR tool by tool.
This is the first one, there are 17 more to come ...
Thanks in advance for the review !