ebi-gene-expression-group / container-galaxy-sc-tertiary

Galaxy container for single cell RNA-Seq tertiary analysis tools
MIT License
16 stars 14 forks source link

Add parameters to Scmap #310

Closed hexhowells closed 2 months ago

hexhowells commented 1 year ago

Defines additional parameters in Scmap Cell Projection that were included recently here: https://github.com/ebi-gene-expression-group/scmap-cli/pull/17

Not sure if anything else needs to be added or modified.

pcm32 commented 1 year ago

Thanks for this @hexhowells . Can you also please change the requirements part to point to the newer version of the package scmap-cli bioconda (0.1.0) and add one or two variations of the test where the new parameters are used (so that we test their usage)? Thanks.

hexhowells commented 1 year ago

Changes should be made now! let me know if there's anything else I need to do.

pcm32 commented 1 year ago

Thanks! Could you please address the warnings and errors of the Linting process:

https://github.com/ebi-gene-expression-group/container-galaxy-sc-tertiary/actions/runs/6942343989/job/18890449183?pr=310

let me know if you need help.

hexhowells commented 10 months ago

Looking at the logs it seems to not be finding the cluster_projection parameter used here . The parameter is specified as a conditional in the xml file which I'm guessing could be a cause of the issue.

https://github.com/ebi-gene-expression-group/container-galaxy-sc-tertiary/actions/runs/6942343989/job/18890449183#step:6:637

+Linting tool /home/runner/work/container-galaxy-sc-tertiary/container-galaxy-sc-tertiary/tools/tertiary-analysis/scmap/scmap_scmap_cell.xml
Failed linting
Applying linter tests... FAIL
.. ERROR: Test 1: Test param cluster_projection not found in the inputs
.. ERROR: Test 2: Test param cluster_projection not found in the inputs

I'm not familiar with the config files used here so I'm not too sure why the parameter isn't being found. The additional test case I wrote was based off of the existing test case in the file which uses the same param so I wouldn't have expected it to throw an error. Some help in figuring out what's causing this would be appreciated! Thanks.

pcm32 commented 9 months ago

Apologies for dropping this. I suspect that the conditional field itself is not a variable, so in the test you probably want to write cluster_projection.do_cluster_projection instead of cluster_projection.

pcm32 commented 9 months ago

Ohh, sorry, I forgot that you have two tests, should be the same for the second one.

hexhowells commented 9 months ago

Okay looking at some other tools in this repo I think I figured out how conditionals are meant to be written for the tests. I think the above commit should fix the linting error.

pcm32 commented 9 months ago

@hexhowells I suspect that the current errors are related to the CI being outdated on this branch. Would you be able to merge back our develop branch to your fork's develop branch. Then this will have the latest CI.

hexhowells commented 9 months ago

@pcm32 I tried merging my fork back and I believe everything is already up to date.

pcm32 commented 9 months ago

Yes, I see now that the base branch wasn't outdated. Not sure why the test-data folder was removed at somepoint. I have found the files being removed on this PR:

https://github.com/ebi-gene-expression-group/container-galaxy-sc-tertiary/pull/213/files

pcm32 commented 9 months ago

Could you try with those test-data files please? If they are very large, please add them to Zenodo and add some logic to bring them in a get_test_data.sh file like this one.

pcm32 commented 9 months ago

@hexhowells there is an option to run planemo to replace the resulting files, are you able to run with that and then replace the resulting test files in your branch?

hexhowells commented 7 months ago

@pcm32 - As this tool update was the result of my internship which has since ended and due to other commitments I am unable to proceed with fixing these issues. Please let me know if this gets updated so that I am able to publish my workflow and tutorial as they rely on this tool.

pcm32 commented 2 months ago

This was all dealt with on https://github.com/ebi-gene-expression-group/container-galaxy-sc-tertiary/pull/318 - so closing this one. Thanks for this great contribution (all your commits merged in the other PR) @hexhowells .