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

add exome flag to exome samples #3061

Merged
merged 26 commits into from
Apr 10, 2024
Merged

Conversation

mathiasbio
Copy link
Contributor

@mathiasbio mathiasbio commented Mar 21, 2024

DONT MERGE UNTIL RELEASE 15 IN BALSAMIC IS VALIDATED AND DEPLOYED

Description

Adds a new argument for WES applications to add the argument --exome to balsamic config case. This will be necessary to merge for the deployment of balsamic release 15: Clinical-Genomics/BALSAMIC#1422

Added

  • --exome argument to balsamic for samples with wes application tags

Changed

Fixed

How to prepare for test

  • Ssh to relevant server (depending on type of change)
  • Use stage: us
  • Paxa the environment: paxa
  • Install on stage (example for Hasta):
    bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_cg -t cg -b [THIS-BRANCH-NAME] -a

How to test

  • install balsamic release 15 to stage feat: Release v15.0.0 BALSAMIC#1422
  • run an exome case and verify that the exome tag has been supplied
  • run a non-exome TGA case and verify that exome tag has NOT been supplied

Expected test outcome

In exome case balsamic config json:

image

In TGA non-exome case balsamic config json:
image

Review

  • Tests executed by
  • "Merge and deploy" approved by
    Thanks for filling in who performed the code review and the test!

This version is a

  • MAJOR - when you make incompatible API changes
  • MINOR - when you add functionality in a backwards compatible manner
  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

  • Document in ...
  • Deploy this branch on ...
  • Inform to ...

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.

🔥

cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
@mathiasbio mathiasbio marked this pull request as ready for review March 25, 2024 07:59
@mathiasbio mathiasbio requested a review from a team as a code owner March 25, 2024 07:59
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.

Well done 💯

tests/meta/workflow/test_balsamic.py Show resolved Hide resolved
cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
Copy link
Contributor

@henrikstranneheim henrikstranneheim left a comment

Choose a reason for hiding this comment

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

Great that you are starting to make PRs in cg, 👍 . I have some comments.

cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
@mathiasbio
Copy link
Contributor Author

Thanks a lot for the reviews! ❤️

Copy link
Contributor

@henrikstranneheim henrikstranneheim left a comment

Choose a reason for hiding this comment

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

Nice, almost there 😄 !

cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
cg/meta/workflow/analysis.py Show resolved Hide resolved
cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
@mathiasbio
Copy link
Contributor Author

Did tests again, and it all still works 👍

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.

💯 🔥

cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
cg/meta/workflow/analysis.py Outdated Show resolved Hide resolved
Copy link
Contributor

@henrikstranneheim henrikstranneheim left a comment

Choose a reason for hiding this comment

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

👍

cg/meta/workflow/balsamic.py Outdated Show resolved Hide resolved
Copy link
Contributor

@henrikstranneheim henrikstranneheim left a comment

Choose a reason for hiding this comment

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

great! 👍

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@mathiasbio mathiasbio merged commit 8284ec4 into master Apr 10, 2024
9 checks passed
@mathiasbio mathiasbio deleted the add_apply_balsamic_exome_tag branch April 10, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

3 participants