Skip to content
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 hitmap wrapper #636

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

GraceAHall
Copy link

HiT-MaP is a tool for analysis of high-resolution MALDI imaging samples.

To pass the single test in the wrapper, a node with 64Gb mem is required.
Attempts have been made to reduce this using an even smaller dataset, but it seems to have little impact on memory usage.

@bgruening
Copy link
Member

@GraceAHall thanks a lot!

Do you know already this project: https://github.com/galaxyproject/galaxy-language-server

This might help you a bit with formatting. Given an input section, it can bootstrap you a test section and a command section. It will show you nice hits and has auto-completion of parameters etc. Moreover, it has common snippets for conditionals, integers ...

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test data is too big, unfortunately. We try to be below 1MB. Can you maybe use the ibd/imzml files from the Cardinal tools?


<command detect_errors="exit_code"><![CDATA[
mkdir expdata &&
cp '${file_inputs.imzml_file}' expdata/sample.imzML &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is ln enough?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunatenly not. Hitmap needs the file extension to be intact, but all galaxy data has extension '.dat'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But instead of using cp you can do ln -l '${file_inputs.imzml_file}' expdata/sample.imzML

That saves a copy operation. And works in 95% of all cases. In some cases, the tool does not follow symlinks ... I hope this is not the case here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm gave this a shot, but since it's a container, it was having difficulty creating the link.

I think this is an improvement which could appear in a later version of the wrapper.

HiTMaP has been very temperamental and hard to get working, so I'd rather just keep as is rather that spending time on this optimization.

@@ -0,0 +1,297 @@

<tool id="hitmap" name="HiT-MaP" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" license="MIT">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the license refers, I think, to code that you ship with this tool not the license of the tool

Can you please also include a profile="20.05"

<import>macros.xml</import>
</macros>

<edam_topics>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you like please include a bio.tools identifier. The edam ontology is not strictly needed anymore, we will get those with the bio.tools IDs. See the xref tag.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HiT-MaP doesn't have a listing in bio.tools. Should I leave this as edam topics?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, leave it! We could also add it to https://github.com/bio-tools/content, but this repo is not yet in production.

cp '${file_inputs.ibd_file}' expdata/sample.ibd &&
cp '${file_inputs.fasta_db}' expdata/database.fasta &&
Rscript '${hitmap_script}' &&
python3 $__tool_directory__/hitmap_gen_html.py expdata &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

single quote for the path

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean $tool_directory/hitmap_gen_html.py -> '$tool_directory/hitmap_gen_html.py'?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes :) the reason is that you might have "spaces" in your tool_directory path .. in this case the tool breaks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha oh yeah that's a great point. Can happen!
Added to the next commit :)


<inputs>
<section name="file_inputs" title="File Inputs" expanded="True">
<param name="imzml_file"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Galaxy datatype imzml is a composite datatype, which means it contains 2 files. You can see how this can be used with the Cardinal tools.

https://github.com/galaxyproteomics/tools-galaxyp/blob/master/tools/cardinal/macros.xml#L18

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I originally tried to do it that way, but couldn't work out how to upload two files (.imzml + .ibd) together as a composite type in the galaxy uploader.

I just went with the approach that the users upload the imzml and ibd as seperate files into their history, then select those when performing the run.

What do you think about that approach?

Also thank you again so much for looking over these submissions and helping me out. I really appreciate it!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about that approach?

I think its can result in more errors from the user side, picking the wrong combination of files etc.
Do you know why this did not work for you? Can you try this on EU. I think its working on EU.

grafik

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhhh I see!
Yes this is working fine, I just wasn't aware of that option. Very neat!
Added to the next commit

@bgruening
Copy link
Member

Hi @GraceAHall! Do we want to work on this one in 2 week? There is a new version on dockerhub: 968278fe1272192cff0f598df5981fd7301d8a4d394370765033409eb3a5053e

@bgruening
Copy link
Member

@GraceAHall I guess we forgot this. Do we still want to do it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants