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

Update PhEval Pipeline Documentation #352

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

Conversation

souzadevinicius
Copy link
Member

Fixes #351

@souzadevinicius souzadevinicius self-assigned this Aug 27, 2024
@souzadevinicius souzadevinicius requested review from matentzn, AO33, kcortes133 and yaseminbridges and removed request for kcortes133 August 27, 2024 04:40
Copy link
Contributor

@yaseminbridges yaseminbridges left a comment

Choose a reason for hiding this comment

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

I had a few comments, but good to see the docs updated, it is definitely more clear now 😄

docs/pipeline.md Show resolved Hide resolved

### Installing Jinja Template
The data preparation phase, checks the completeness of the disease, gene and variant input data and optionally preparing simulated VCF files if required, gives the user the ability to randomise phenotypic profiles using the PhEval corpus scramble command utility, allowing for the assessment of how well VGPAs handle noise and less specific phenotypic profiles when making predict.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should clarify that the checking of the completeness of the disease, gene & variant input data is in fact checking the data contained in the phenopacket is complete for these fields. Also, maybe this paragraph could be split just so that it flows a bit better.

docs/pipeline.md Show resolved Hide resolved

In resources folder, there is a file named *pheval-config.yaml*, this file is responsible for storing the PhEval Makefile generation.
- `data` property refers to the folder location where the necessary phenotypic data for the pipeline will be downloaded and extracted.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the data property specific to Exomiser? What will happen if it is left blank and users don't want to run Exomiser?

docs/pipeline.md Show resolved Hide resolved
docs/pipeline.md Show resolved Hide resolved
@julesjacobsen
Copy link
Contributor

julesjacobsen commented Sep 5, 2024

I would say that the Pipeline docs all belong in the monarch_pheval repo? At the very least the About section should come before the Pipeline section as there is absolutely no context about what the pipeline is even for if someone is reading the docs through from the start.

In fact, reading through the docs from the start still gives me no help in telling me how I might want to use PhEval or what it can do. A few simple examples in a logical set of steps explaining the library and the plugins to a complete beginner to lead them through to being able to write a plugin and execute a benchmark is what is needed.

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.

Update PhEval Pipeline Documentation
3 participants