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

fix: command in vcf2cytosure rule and update ReadtheDocs #966

Merged
merged 15 commits into from
Jun 23, 2022

Conversation

khurrammaqbool
Copy link
Collaborator

This PR:

Changed: ReadtheDocs
Fixed: command in vcf2cytosure rule

Review and tests:

  • Tests pass
  • Code review
  • New code is executed and covered by tests, and test approve

@codecov
Copy link

codecov bot commented Jun 22, 2022

Codecov Report

Merging #966 (0343a09) into develop (6df59a8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #966   +/-   ##
========================================
  Coverage    99.24%   99.24%           
========================================
  Files           29       29           
  Lines         1714     1714           
========================================
  Hits          1701     1701           
  Misses          13       13           
Flag Coverage Δ
unittests 99.24% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6df59a8...0343a09. Read the comment docs.

@khurrammaqbool khurrammaqbool mentioned this pull request Jun 22, 2022
17 tasks
Copy link
Contributor

@ivadym ivadym left a comment

Choose a reason for hiding this comment

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

The changes regarding docs looks fine, but it still doesnt render the CLI https://balsamic-docs.readthedocs.io/en/latest/cli_package.html (I tested it in my fork). I will further look into this.

Regarding the content, we discussed some names but maybe you @ashwini06 have some better suggestions how the structure of references/tools should look.

@ashwini06
Copy link
Contributor

Regarding the content, we discussed some names but maybe you @ashwini06 have some better suggestions how the structure of references/tools should look.

@ivadym @khurrammaqbool : I propose that we can change the existing balsamic readthedocs to a more structured one. How about the following structure?

Getting Started

  • Installation
  • Short tutorial

Detailed Documentation:

  • (BALSAMIC) Annotation Resources
  • (BALSAMIC) Variant Calling Algorithms
  • (BALSAMIC) Method descriptions
  • (BALSAMIC) Tools and Softwares
  • (BALSAMIC) CLI usage
  • CHANGELOG

Development Guide

  • Git Etiquette
  • Snakemake Etiquette
  • Build doc
  • Semantic Versioning
  • FAQs

OTHER INFO:

  • Knowledge base and Resources

@ivadym
Copy link
Contributor

ivadym commented Jun 22, 2022

The suggested names look good! We have some new entries in develop branch that I included below and me and @khurrammaqbool liked this structure:

Getting Started

  • Installation
  • Short tutorial
  • CLI usage

Detailed Documentation:

  • Calling and filtering variants
  • Structural and Copy number Variants
  • Annotation resources
  • Method descriptions
  • Tools and Software
  • Changelog
  • References

Development Guide

  • Git Etiquette
  • Snakemake Etiquette
  • Build doc
  • Semantic Versioning
  • FAQs

@ashwini06
Copy link
Contributor

  • References

The suggested names look good! We have some new entries in develop branch that I included below and me and @khurrammaqbool liked this structure:

Getting Started

  • Installation
  • Short tutorial
  • CLI usage

Detailed Documentation:

  • Calling and filtering variants
  • Structural and Copy number Variants
  • Annotation resources
  • Method descriptions
  • Tools and Software
  • Changelog
    > * References

Development Guide

  • Git Etiquette
  • Snakemake Etiquette
  • Build doc
  • Semantic Versioning
  • FAQs

References: References doesn't sound like a correct term here, I would suggest keeping this outside of detailed documentation (like Other info), as it is mostly a Knowledge base resource and the text within it includes a huge collection of tools from various resources.

@ashwini06
Copy link
Contributor

@ivadym : Don't forget to add new links to the delivery report, as these Variant Calling Algorithms link will be broken :)

Copy link
Contributor

@ashwini06 ashwini06 left a comment

Choose a reason for hiding this comment

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

few suggestions

- Analysis type
- Somatic/Germline
- Variant type
* - DNAScope
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* - DNAScope
* - DNAscope

- tumor-normal, tumor-only
- somatic
- SNV, InDel
* - TNScope :superscript:`2`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* - TNScope :superscript:`2`
* - TNscope :superscript:`2`

- tumor-normal, tumor-only
- somatic
- SNV, InDel
* - TNScope_umi
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* - TNScope_umi
* - TNscope_umi


:superscript:`1` TNhaplotyper is only executed for tumor-only if a WGS case is being analysed

:superscript:`2` TNScope output is being merged with TNhaplotyper calls for TO-WGS analysis
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:superscript:`2` TNScope output is being merged with TNhaplotyper calls for TO-WGS analysis
:superscript:`2` TNscope output is being merged with TNhaplotyper calls for TO-WGS analysis

@@ -1,5 +1,5 @@
************************************
Structural and Copy Number Variants
Structural and copy-number variants
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Structural and copy-number variants
Structural and Copy Number variants

CHANGELOG.rst Outdated
@@ -37,6 +37,7 @@ Fixed:
* `run_validate.sh` script https://github.com/Clinical-Genomics/BALSAMIC/pull/952
* Somatic SV tumor normal rules https://github.com/Clinical-Genomics/BALSAMIC/pull/959
* Missing `genderChr` flag for `ascat_tumor_normal` rule https://github.com/Clinical-Genomics/BALSAMIC/pull/963
* Command in vcf2cyosure rule and updated ReadtheDocs https://github.com/Clinical-Genomics/BALSAMIC/pull/966
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Command in vcf2cyosure rule and updated ReadtheDocs https://github.com/Clinical-Genomics/BALSAMIC/pull/966
* Command in vcf2cytosure rule and updated ReadtheDocs https://github.com/Clinical-Genomics/BALSAMIC/pull/966

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@khurrammaqbool khurrammaqbool merged commit c684d44 into develop Jun 23, 2022
@khurrammaqbool khurrammaqbool deleted the fix/command_in_vcf2cytosure_rule branch June 23, 2022 08:44
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.

3 participants