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
Added Cami Opal - A tool for evaluating taxonomic metagenome profilers #6096
base: main
Are you sure you want to change the base?
Added Cami Opal - A tool for evaluating taxonomic metagenome profilers #6096
Changes from 9 commits
33a53eb
461a8a7
3db4745
d15df70
a09b364
fbd0a54
5a195bb
d462694
0f22502
15dea3a
047becc
a7d5b4b
060f3bf
fde7a42
f01b119
be596e9
75cb130
d984487
f40b015
76a72b1
ae15a9a
d21db03
edd2a57
21ef9db
080c7a3
e708085
4361fe2
db4cc5c
c7a4d49
7e05dd0
7146070
8cffbbb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
#if $html_output
should workThere 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.
Try to avoid redirect stderr. It makes finding bugs more difficult.
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 kind of run times and memory settings are those?
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.
Thank you, @bernt-matthias, for your time and attention in reviewing the tool configuration.
The
-t/--time
and-m/--memory
options in the tool are designed to log computational resources, such as runtimes in hours and memory usage in gigabytes. Opal Arguments SectionAs I aim to adhere to Galaxy standards and best practices, could you advise if there are specific guidelines or standards I should consider for these options?
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 see. So the two parameters are supposed to list the runtimes and used memory that have been used to compute the profiles?
My first question is if this will be used? Technically you can find this out in Galaxy (manually), but still the question is if this is useful information.
If so, it might be an option to keep the repeat and add a runtime and memory parameter in the repeat. Then the assignment between profile, memory, and time is clearer.
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
-t/--time
and-m/--memory
parameters inopal.py
require manual entry of runtimes and memory usage, which Galaxy already tracks automatically - am I right? I find these manual entries thereby a bit redundant. Do you think we still need to implement this function?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 allow adding it as optional, one could get this info from galaxy and add manually
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.
If you make this a select it would be more user friendly.
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.
Also here a select would be better... same for all arguments with a fixed set of possible options.