phnmnl / container-galaxy-k8s-runtime

PhenoMeNal runtime for Galaxy running inside a container orchestrator
Apache License 2.0
5 stars 18 forks source link

New nmr-integrals tool for NMR spectrum comparison #223

Closed ilveroluca closed 6 years ago

ilveroluca commented 6 years ago

This is a tool from CERM to compare a reference spectrum to a newly acquired one. I'd really like to get it into this release so here's the PR on the day of the deadline :-)

The implementation works, though the Galaxy integration is iffy (but I can fix it during our bug fixing phase).

In addition to the tool, this PR introduces the bruker_zip galaxy data type for a zip archived Bruker NMR output. If uploaded the dataset is not extracted by Galaxy.

michaelvanvliet commented 6 years ago

@ilveroluca I'm not to sure about the datatype, bruker_zip is too generic as Bruker does more than just NMR, and probably there are several versions of the zipped output by Bruker. Why not just use the format="no_unzip.zip,zip" See: https://github.com/phnmnl/container-galaxy-k8s-runtime/blob/feature/mzquality/tools/phenomenal/ms/mzquality/ms-vfetc.xml , here it also doesn't unzip when to give it the no_unzip type while uploading.

ilveroluca commented 6 years ago

Thanks for the useful review @michaelvanvliet. My reasoning regarding the datatype is that one could hypothetically zip any number of things and hand them to the wrong tool. A more specific data type would help avoid mixing different types of zip archives within the context of a Galaxy workflow or analysis session.

However, I didn't know that there are multiple types of (incompatible) datasets deriving from Bruker NMR machines. Given your feedback, I'll get rid of the bruker_zip data type, but I do think that it would be a good idea to have explicit data types for the various types of zipped datasets that we handle (moreover, it's trivial to create them with a single new element in datatypes_conf.xml).

c-ruttkies commented 6 years ago

Shall I prepare a release built of the container so that we can add a proper release tag for the container description in config/phenomenal_tools2container.yaml ?

ilveroluca commented 6 years ago

@michaelvanvliet could you review again please? Following a conversation with my users I made a change to the interface.