Import (Bruker NMR data) fails

[rrueedi]

With error

2018-07-26T13:04:28.224165487Z python: can't open file '/export/tools/phenomenal/nmr/tamenmr/import2csv.py': [Errno 2] No such file or directory

[c-ruttkies]

With a quick look at the Dockerfile I guess the problem is here:

for i in $(find tameNMR/tameNMR -name *.R); do install -m755 $i /usr/local/bin; done

This command only puts R-Scripts in the PATH. Python scripts aren't put there.

[c-ruttkies]

I don't find the python scripts used in ident.xml and quant.xml. Is this an issue @ArturasG ?

[c-ruttkies]

I added several test scripts https://github.com/phnmnl/container-tamenmr/tree/hotfix/py-scripts/runTest running the commands from https://github.com/PGB-LIV/tameNMR/tree/master/test/testScripts that are also used for the galaxy wrappers. As several tools are not passing we need to exclude it from the release. I also tested a few galaxy tools which showed problems.
Please also refer to https://github.com/PGB-LIV/tameNMR/issues

I would be happy to help fixing the issues.

[ArturasG]

I was a bit out of the loop on this in the last few months and completely missed this email among all the phnmnl traffic. It's unfortunate that it didn't end up in the release. Is there any chance of it to be added later or is the current version final? The required changes should be minimal (surprising there are any) as I have a working version of this on our local server.

…

On Wed, Aug 1, 2018 at 3:05 PM c-ruttkies ***@***.***> wrote: I added several test scripts running the commands https://github.com/PGB-LIV/tameNMR/tree/master/test/testScripts that are also used for the galaxy wrappers. As several are not passing we need to exclude it from the release. I also tested the few galaxy tools. Please also refer to https://github.com/PGB-LIV/tameNMR/issues I would be happy to help with fixing the issues. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG37nTGTyND0e9c_DbaNTqyANi21Vxf0ks5uMbXEgaJpZM4VhzsS> .

[ilveroluca]

Hi Arturas, If you can have the update ready in short order it should be possible. Luca Il giorno dom 2 dic 2018, 14:57 Arturas Grauslys <[email protected]> ha scritto:

…

I was a bit out of the loop on this in the last few months and completely missed this email among all the phnmnl traffic. It's unfortunate that it didn't end up in the release. Is there any chance of it to be added later or is the current version final? The required changes should be minimal (surprising there are any) as I have a working version of this on our local server. On Wed, Aug 1, 2018 at 3:05 PM c-ruttkies ***@***.***> wrote: > I added several test scripts running the commands > https://github.com/PGB-LIV/tameNMR/tree/master/test/testScripts that are > also used for the galaxy wrappers. As several are not passing we need to > exclude it from the release. I also tested the few galaxy tools. > Please also refer to https://github.com/PGB-LIV/tameNMR/issues > > I would be happy to help with fixing the issues. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #2 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AG37nTGTyND0e9c_DbaNTqyANi21Vxf0ks5uMbXEgaJpZM4VhzsS > > . > — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABbu2BJFvth9F6NTIORjJSY7UEXuK1Rqks5u09wwgaJpZM4VhzsS> .

[ArturasG]

Hi Luca, thanks for a quick reply. I have looked through the errors in the tests and they were mostly due to an old test file that was comma-delimited where currently it is required to be tab-delimited. The "libPaths" problem was a leftover line from my local version that was overlooked. I removed it and added a shebang that was missing. I also created a separate repository for the tameNMR-phenomenal version so there are no accidental clashes with our local version and it can be updated as needed. I have edited the Dockerfile to point to that repository instead. My data import tool was giving me problems since automatic unzipping in my local server is disabled and we use zip files to upload multiple files to the server. Since you have BATMAN on already and all my spectra processing tools require the same input format I assume there is already a way to get the data to that format therefore I opted to remove the import script as redundant. I have edited the config files in the galaxy to exclude it. ( I assume I need to submit a pull request for the runtime container for this change to be included?) I have also edited a couple of xml files' description section to add clarity. I hope this is OK. Please let me know further steps. Many thanks Arturas On Sun, Dec 2, 2018 at 2:06 PM Luca Pireddu <[email protected]> wrote:

…

Hi Arturas, If you can have the update ready in short order it should be possible. Luca Il giorno dom 2 dic 2018, 14:57 Arturas Grauslys ***@***.*** > ha scritto: > I was a bit out of the loop on this in the last few months and completely > missed this email among all the phnmnl traffic. It's unfortunate that it > didn't end up in the release. Is there any chance of it to be added later > or is the current version final? The required changes should be minimal > (surprising there are any) as I have a working version of this on our local > server. > > On Wed, Aug 1, 2018 at 3:05 PM c-ruttkies ***@***.***> > wrote: > > > I added several test scripts running the commands > > https://github.com/PGB-LIV/tameNMR/tree/master/test/testScripts that are > > also used for the galaxy wrappers. As several are not passing we need to > > exclude it from the release. I also tested the few galaxy tools. > > Please also refer to https://github.com/PGB-LIV/tameNMR/issues > > > > I would be happy to help with fixing the issues. > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > < > #2 (comment) > >, > > or mute the thread > > < > https://github.com/notifications/unsubscribe-auth/AG37nTGTyND0e9c_DbaNTqyANi21Vxf0ks5uMbXEgaJpZM4VhzsS > > > > . > > > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > < #2 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABbu2BJFvth9F6NTIORjJSY7UEXuK1Rqks5u09wwgaJpZM4VhzsS > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG37nSPW9GOhMxdmp_kInWm2MfEunqS3ks5u095LgaJpZM4VhzsS> .

[ilveroluca]

Hi Arturas. I'm afraid one of the main issues with this container was the fact that it's missing tests (maybe @c-ruttkies, who helped with PR #315, remembers better). We need the runTest1.sh script to implement an automatic test that fetches one or more datasets, runs the tool and verifies that the output is as expected. If you look at runTest1.sh in container-tamenmr you'll see that it merely loads a library.

Here's a relatively simple runTest1.sh you could use as a model:
https://github.com/phnmnl/container-nmr-integrals/blob/master/runTest1.sh

Do you think we could implement a data-based test for tamenmr? If you need help with some things (e.g., where to put data sets so that they can be downloaded by the test) let us know.

Luca

[korseby]

I bring myself into the loop :)

[ilveroluca]

👍 :-)

[ArturasG]

Hi Luca, thanks for the info. I was not aware of the requirement. I'll implement the tests as soon as I have time (most likely right after the holiday break). Many thanks Arturas

…

On Tue, Dec 18, 2018 at 10:02 AM Luca Pireddu ***@***.***> wrote: 👍 :-) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG37nQK4CljuGuhwr7ehaHA2z0eWb84oks5u6L1TgaJpZM4VhzsS> .

[ilveroluca]

No problem. This test will enable continuous integration testing of the container, meaning that any change pushed to the repository's main branches (usually master and develop) will be tested automatically by Jenkins before the new image is made available.

Happy holidays :-)

[ArturasG]

Hi Luca,

I have at last found some time to implement the automated tests for tameNMR (and a few other fixes). I have also made a separate repository for a tameNMR version for PhenoMeNal so it doesn't clash with out local version. I have build the Docker container from the current develop branch and ran the tests from within the container. Everything seems to run fine.

What next steps do I need to take?

Many thanks