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

Extensive IGV Name Parameter #316

Merged
merged 12 commits into from
Feb 8, 2024
Merged

Extensive IGV Name Parameter #316

merged 12 commits into from
Feb 8, 2024

Conversation

williamputraintan
Copy link
Member

@williamputraintan williamputraintan commented Feb 7, 2024

  • Added format of igv name parameter to {subjectId}_{type}_{libraryId}_filename.bam for both IGV local and js
  • Update reference genome for IGV js

Ref: Slack - Threads

@williamputraintan williamputraintan self-assigned this Feb 7, 2024
@williamputraintan williamputraintan added the feature New feature or request label Feb 7, 2024
@williamputraintan williamputraintan added this to the Release 2.2.1 milestone Feb 7, 2024
@victorskl
Copy link
Member

Reviewing...

Copy link
Member

@victorskl victorskl left a comment

Choose a reason for hiding this comment

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

LGTM. Minor comment to attend before merging. Pls request re-review whenever you are ready, pls.

* Ref: https://umccr.slack.com/archives/CP356DDCH/p1707116441928299?thread_ts=1706583808.733149&cid=CP356DDCH
*
* The desired outcome is to include libraryId, sampleId, type, and filetype
* Desired output: SBJ00000_WGS_L0000000_PRJ00000_tumor.bam
Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry Will. Spec change a bit. Could you remove sample type field, pls?

It should just be the SubjectID_LibraryID_ prepending only.

e.g. SBJ00000_L0000000_PRJ00000_tumor.bam

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry just thought I could go for an extra mile here to include the type. But, yes could follow what we agree on slack. Changing it ...

indexed: false,
visibilityWindow: -1,
removable: false,
order: 1000000,
infoURL: 'https://www.ncbi.nlm.nih.gov/gene/?term=$$',
Copy link
Member

Choose a reason for hiding this comment

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

Oh, these igv.js reference genomes have changed. Nice update there, Will.!

It seems to load a tad quicker than previous ones. Nice. (FYI @jrobinso). Was thinking mirroring local region ap-southeast with #87

nameArray.push(...findMatchingProperty('library_id'));

// 3. sampleId + filetype
nameArray.push(pathOrKey.split('/').pop() ?? pathOrKey);
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to discuss the logic a little bit here. Perhaps, we catchup tomorrow.

Especially for BAM case, we might not want to match library_id from the path. The path pertaining two library_id has other meaning as they belong to a single workflow run.

However, a BAM file only represents 1 sample; either tumor or normal. So, we need to extract sample_id from the filename; strip tumor or normal (if exists); then using this sample_id to exchange its library_id counterpart from the subject meta/lims. So it is to be precise. It will be rather filter (or lookup) logic than reducing with accumulator on all possible library_id matches infer from the path.

What you did here is; arguable not completely wrong either - especially when we switch to VCF context. But then, that open up even more complex handling where I'd rather ignore prefix for VCF all together. Only support BAM case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made the change so only bam that will have the SBJ00000_L0000000_PRJ00000_tumor.bam format while other filetype will only prepend the subjectId

Copy link
Member

@victorskl victorskl left a comment

Choose a reason for hiding this comment

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

Looking great, Will! Let push it through to Staging.

@williamputraintan williamputraintan merged commit eff89b3 into dev Feb 8, 2024
1 check passed
@williamputraintan williamputraintan deleted the igv-update branch February 8, 2024 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants