-
Notifications
You must be signed in to change notification settings - Fork 28
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 XCMS plotting tools #267
Conversation
@lecorguille think it should be fixed now :) Thanks for reviewing! |
@lecorguille @bgruening ping |
@hechth tests are failing... |
It complains about maximum file size but all added files are tiny @lecorguille and @bgruening |
LGTM! |
tools/xcms/xcms_plot_eic.xml
Outdated
<param name="mz_value" value="153.06614"/> | ||
<param name="tolerance_ppm" value="10"/> | ||
<param name="mslevel" value="1"/> | ||
<output name="output_filename" file="eic_plot.png"/> |
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.
Keep in mind that with the latest Galaxy we have all those nice asserts that are better than diif or sim_size comparisons: https://docs.galaxyproject.org/en/latest/dev/schema.html#assertions-for-image-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.
Ah thanks - that is good to know. This is our first plotting tool so wasn't familiar with 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.
I really can't figure out why the tests fail here on the CI - do you think it would make sense to remove the files all together and only use those assertions?
Test are failing here. |
Test is failing due to file size but that is invalid ... I don't really know what else to do here. Maybe the CI is outdated? The tests pass locally - should I try opening the same PR against tools-iuc and see what happens? |
Can you maybe update the ci? |
Hi can we please try to move this forward? We'd really like to use the tools and I'd hate to move them again back over to our repo to speed up the process. |
Hi, can you please trigger the CI @lecorguille and @bgruening |
done |
FOR CONTRIBUTOR:
See RECETOX/galaxytools#523 for xref