search

monarch-initiative / pheval.exomiser

Exomiser plugin for PhEval.
4 stars 1 forks source link

#3 add option to run exomiser with hpo terms only #8

Closed yaseminbridges closed 1 year ago

yaseminbridges commented 1 year ago

Implemented an option to run Exomiser with --preset phenotype-only which only takes into account HPO terms when running analysis (no vcf required). This option should only be run with Exomiser version 13.2.0 onwards as previous versions have a much longer computational time for computing these results.

Methods dealing with new Exomiser 13.2.0 features, e.g. specification of results directory and file prefix, have also been implemented while retaining previous methods that are compatible with previous versions of Exomiser.

yaseminbridges commented 1 year ago

Removed the manual path setting throughout the code for methods that are implemented in the runner. Refactored some variable names for consistency with the directory naming in PhEval.

There shouldn't be any conflict now with the differences in the directory naming that @souzadevinicius mentioned in the meeting as I've removed the creation of these specific paths with the understanding that it is going to be handled by the MakeFile, i.e. the path that I am expecting to be passed to pheval run is --output-dir ./exomiser-13.2.0/default-corpus1-default .

Ultimately, this PR was to add in the option to run with the phenotype-only preset in Exomiser for a clearer comparison with Phen2Gene/other gene prioritisation softwares, so I have left in the reliance on the runner specific config etc for now but I know this is something that may be changed later

yaseminbridges commented 1 year ago

Still that problem? Is it a good idea to comment out all tests?

@matentzn I can't reply to this directly. These tests were used to test the CLI runner, I don't see how these could be implemented now that I have overwritten the methods within the Exomiser Runner. I'd need access to the exomiser data for it all to run without error which isn't possible. The other test checked if an error is raised when an invalid runner is passed but I don't think that's a test that should be done by a plugin but rather tested in PhEval instead

matentzn commented 1 year ago

Sorry I thought we were in pheval repo! You are right ignore my comment.

yaseminbridges commented 1 year ago

Sorry I thought we were in pheval repo! You are right ignore my comment.

I will delete the file instead of hashing it all out to avoid confusion