Conversation
…odel artifacts; modified argument parser and pvacseq run.py file to run the predictor as part of pvacseq; added test files and script.
jyao36
changed the title
There was a problem hiding this comment.
This looks pretty good. I added some comments for improvements that could be made.
I would also like to see the addition of a standalone pvacseq add_ml_predictions command that someone could run standalone to add ml_predictions to the output from their pipeline if the ml option was not enabled in the original run. Have a look at pvactools/tools/pvacseq/mark_genes_of_interest.py and its test tests/test_pvacseq_mark_genes_of_interest.py to see how to do so. This command addition also requires an update to pvactools/tools/pvacseq/main.py and pvactools/tools/pvacseq/__init__.py to add this command.
Additionally, this new command should then be documented in docs/pvacseq/optional_downstream_analysis_tools.rst. Most of the documentation can be auto-generated from the command line docs by using the program-output tool but this would also be the spot to put any additional documentation you want to have around this feature.
susannasiebert
left a comment
susannasiebert
left a comment
There was a problem hiding this comment.
Left a couple of code suggestions to convert named required parameters to positional parameters in your parser.
Co-authored-by: Susanna Kiwala <susanna.kiwala@wustl.edu>
Co-authored-by: Susanna Kiwala <susanna.kiwala@wustl.edu>
Co-authored-by: Susanna Kiwala <susanna.kiwala@wustl.edu>
…or; updated argument format for add_ml_predictions.py
...ml_predictor/ml_predictor_test_output/HCC1395.MHC_I.all_epitopes.aggregated.ML_predicted.tsv
Show resolved
Hide resolved
susannasiebert
left a comment
susannasiebert
left a comment
There was a problem hiding this comment.
I made some copy editing suggestions. Two thoughts I had while reviewing this:
- It would be great to have some additional exploration of different candidates in ML section of the pVACview vignette. I wrote down some examples in a comment in that part of the text.
- Have you and Malachi discussed whether it would be worthwhile to make the reject threshold configurable? It seems a bit weird to me to hardcode the reject threshold but make the accept threshold configurable.
| * - ``ui.R``, ``app.R``, ``server.R``, ``styling.R``, ``anchor_and_helper_functions.R`` | ||
| - pVACview R Shiny application files. Not generated when running only with presentation and immunogenicity algorithms. | ||
| * - ``www`` (directory) | ||
| - Directory containing image files for pVACview. Not generated when running with presentation and immunogenicity algorithms only. | ||
| * - ``ml_predict/<sample_name>_predict_pvacview.tsv`` (optional) | ||
| - ML-based neoantigen evaluation predictions file. Generated when both MHC Class I and Class II predictions are run and the ``--run-ml-predictions`` flag is set. | ||
| - Directory containing image files for pVACview. Not generated when running only with presentation and immunogenicity algorithms only. | ||
|
|
There was a problem hiding this comment.
This table doesn't seem to be showing up on the webpage
docs/pvacseq/output_files.rst
Outdated
| - Directory containing image files for pVACview. Not generated when running with presentation and immunogenicity algorithms only. | ||
| * - ``ml_predict/<sample_name>_predict_pvacview.tsv`` (optional) | ||
| - ML-based neoantigen evaluation predictions file. Generated when both MHC Class I and Class II predictions are run and the ``--run-ml-predictions`` flag is set. | ||
| - Directory containing image files for pVACview. Not generated when running only with presentation and immunogenicity algorithms only. |
There was a problem hiding this comment.
| - Directory containing image files for pVACview. Not generated when running only with presentation and immunogenicity algorithms only. |
This should fix the issue with this table not rendering.
… columns and fill with NaN; Updated ML file name, argument names; Updated documentation
2 participants
…odel artifacts; modified argument parser and pvacseq run.py file to run the predictor as part of pvacseq; added test files and script.