TASK-6324 - Simplify QC analysis by launching a single job#2469
Open
jtarraga wants to merge 25 commits intorelease-3.x.xfrom
Open
TASK-6324 - Simplify QC analysis by launching a single job#2469jtarraga wants to merge 25 commits intorelease-3.x.xfrom
jtarraga wants to merge 25 commits intorelease-3.x.xfrom
Conversation
…b that executes sequentially samtools, plot-bamstats and fastqc, #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentFastQcMetricsAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/executors/DockerWrapperAnalysisExecutor.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/samtools/SamtoolsWrapperAnalysisExecutor.java modified: opencga-analysis/src/test/java/org/opencb/opencga/analysis/alignment/AlignmentAnalysisTest.java modified: opencga-core/src/main/java/org/opencb/opencga/core/tools/OpenCgaToolExecutor.java modified: opencga-core/src/main/java/org/opencb/opencga/core/tools/result/ExecutionResultManager.java
…TASK-6324 On branch TASK-6324 Changes to be committed: new file: opencga-app/app/analysis/genome-plot/circos.R
…nused parameters, #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/fastqc/FastqcWrapperAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/fastqc/FastqcWrapperAnalysisExecutor.java
… unused parameters, #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/samtools/SamtoolsWrapperAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/samtools/SamtoolsWrapperAnalysisExecutor.java
…tools stats/flagstats, fastqc), #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
…r sonnar issues, #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/test/java/org/opencb/opencga/analysis/alignment/AlignmentAnalysisTest.java
…pos and sonnar issues, #TASK-6326, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/mutationalSignature/MutationalSignatureAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/mutationalSignature/MutationalSignatureLocalAnalysisExecutor.java modified: opencga-core/src/main/java/org/opencb/opencga/core/tools/variant/MutationalSignatureAnalysisExecutor.java
On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/stats/SampleVariantStatsAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/stats/SampleVariantStatsLocalAnalysisExecutor.java modified: opencga-core/src/main/java/org/opencb/opencga/core/tools/variant/SampleVariantStatsAnalysisExecutor.java
…lot analysis), fix some typos and sonnar issues, #TASK-6326, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/genomePlot/GenomePlotAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/genomePlot/GenomePlotLocalAnalysisExecutor.java modified: opencga-core/src/main/java/org/opencb/opencga/core/tools/variant/GenomePlotAnalysisExecutor.java
On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
…e QC step, #TASK-6326, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java
…ASK-6324 On branch TASK-6324 Changes to be committed: deleted: opencga-analysis/src/main/R/genome-plot/circos.R deleted: opencga-analysis/src/main/R/mutational-signature/mutational-signature.r modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
…lysis folder, #TASK-6326, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/circos/CircosLocalAnalysisExecutor.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/genomePlot/GenomePlotLocalAnalysisExecutor.java
On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/test/java/org/opencb/opencga/analysis/variant/OpenCGATestExternalResource.java modified: opencga-analysis/src/test/java/org/opencb/opencga/analysis/variant/VariantAnalysisTest.java deleted: opencga-storage/opencga-storage-core/src/test/resources/AR2.10039966-01T.copynumber.caveman.vcf.gz deleted: opencga-storage/opencga-storage-core/src/test/resources/AR2.10039966-01T_vs_AR2.10039966-01G.annot.brass.vcf.gz deleted: opencga-storage/opencga-storage-core/src/test/resources/AR2.10039966-01T_vs_AR2.10039966-01G.annot.pindel.vcf.gz new file: opencga-storage/opencga-storage-core/src/test/resources/cancer-cnvs.vcf.gz new file: opencga-storage/opencga-storage-core/src/test/resources/cancer-indels.vcf.gz new file: opencga-storage/opencga-storage-core/src/test/resources/cancer-rearrs.vcf.gz new file: opencga-storage/opencga-storage-core/src/test/resources/cancer-snvs.vcf.gz new file: opencga-storage/opencga-storage-core/src/test/resources/genome-plot-config.json
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| logger.info("Signagture fit max. rare sigs.: {}", signatureParams.getFitMaxRareSigs()); | ||
| logger.info("Signagture fit signatures file: {}", signaturesFile); | ||
| logger.info("Signagture fit rare signatures file: {}", rareSignaturesFile); | ||
| logger.info("Signature id: {}", signatureParams.getId()); |
Check notice
Code scanning / SonarCloud
Logging should not be vulnerable to injection attacks
| logger.info("Signagture fit signatures file: {}", signaturesFile); | ||
| logger.info("Signagture fit rare signatures file: {}", rareSignaturesFile); | ||
| logger.info("Signature id: {}", signatureParams.getId()); | ||
| logger.info("Signature description: {}", signatureParams.getDescription()); |
Check notice
Code scanning / SonarCloud
Logging should not be vulnerable to injection attacks
j-coll
requested changes
Jun 19, 2024
| throw new ToolException("Error copying Samtools flagstat results", e); | ||
| } | ||
| } else { | ||
| throw new ToolException("Something wrong happened running Samtools flagstat analysis"); |
Member
There was a problem hiding this comment.
Your future self will appreciate if you add some extra information here. Like the number of lines, and/or the actual first line
| } | ||
| } else { | ||
| String msg = "Skipping sample variant stats analysis by user"; | ||
| addWarning(msg); |
Member
There was a problem hiding this comment.
Is this even a warning, or should it be just an info event?
| erm.addAttribute(key, value); | ||
| } | ||
|
|
||
| protected final void addStepAttributes(ExecutionResult executionResult) throws ToolException { |
Member
There was a problem hiding this comment.
You might want to include more information from the nested ExecutionResult, like events (maybe that's the only interesting thing)
Member
Author
There was a problem hiding this comment.
do you mean to add the "nested" ExecutionResult events into the list of "parent" Execution events?,
Member
There was a problem hiding this comment.
Yes, something like that. Otherwise, they're lost (not technically, as you can always read the nested job result file, but they'd be quite hidden).
… #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
…-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java modified: opencga-core/src/main/java/org/opencb/opencga/core/tools/result/ExecutionResultManager.java
…as wrong running dockers (samtools stats/flagstats and fastqc), #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentFastQcMetricsAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/wrappers/executors/DockerWrapperAnalysisExecutor.java modified: opencga-analysis/src/test/java/org/opencb/opencga/analysis/alignment/AlignmentAnalysisTest.java
j-coll
reviewed
Jun 20, 2024
| protected void run() throws ToolException { | ||
| // Create the tool runner | ||
| toolRunner = new ToolRunner(opencgaHome, catalogManager, StorageEngineFactory.get(variantStorageManager.getStorageConfiguration())); | ||
| toolRunner = new ToolRunner(getOpencgaHome().toString(), catalogManager, |
Member
There was a problem hiding this comment.
You can better use the constructor
public ToolRunner(String opencgaHome, CatalogManager catalogManager, VariantStorageManager variantStorageManager)
instead of
ToolRunner(String opencgaHome, CatalogManager catalogManager, StorageEngineFactory storageEngineFactory)
… attributes, #TASK-6325, #TASK-6324 On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/tools/OpenCgaTool.java
On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/alignment/qc/AlignmentQcAnalysis.java
On branch TASK-6324 Changes to be committed: modified: opencga-analysis/src/main/java/org/opencb/opencga/analysis/sample/qc/SampleQcAnalysis.java
jtarraga
changed the title
j-coll
previously approved these changes
Jun 25, 2024
j-coll
changed the title
Contributor
juanfeSanahuja
approved these changes
Aug 14, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TASK-6324 Simplify QC analysis by launching a single job:
Alignment QC analysis
Sample QC analysis