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

trans-interaction display and arcs dataset to circular view #1447

Merged
merged 7 commits into from
Jan 5, 2022

Conversation

cheehongsg
Copy link
Contributor

@jrobinso : This a followup on trans-interaction rendering and arc dataset passing and recording for circular view.

  1. The trans-interaction rendering has its fill style set to chromosome color of mate chromosome in the interaction feature instead of the track color.
    This will allow user to get chromosome specific characteristics from the linear view. User may get the same information from the circular view, albeit a different user experience.

This additional coloring by mate chromosome triggers errors when user uses menu "Save SVG". The causes are identified as such:
1.1) Missing closing parenthesis in chrY color definition (chrColorMap in js/bam/bamTrack.js).

1.2) By mate chromosome id which is not in chrColorMap. (E.g. chrM) IGVColor.randomRGB() is invoked to assigned a random color in getChrColor(), but the expected color range min and max are missing.

  1. Allow user to additively build up the circular view via "add interactions to circular view" from the track viewport. Having the same data track/dataset name means the arcs get replaced while we select the same option in different locus sequently, losing the additive effect.

2.1) Added the genomic region to the name of the arc dataset send over to circular view to allow dataset not to be overwritten.

2.2) The data range filter the arc in the linear view. We pass on the effect of this filtering to circular view. Thus, this information is also embedded in the name of the arc dataset send over to the circular view.

  1. We previously discuss merging drawProportionalChIAPET() into drawProportional(). Here are the differences to consider during the merging.

3.1) drawProportionalChIAPET() caches the rendered arcs and prevent redrawing visually identical arcs. Points are calculated as integer rather than float to minimise the memory requirement.e

3.2) To maintain the same visual effect for the interaction score, y-scaling is still handle from [0, max] even when dataRange.min>0

3.3) drawProportionalChIAPET() handle trans interaction differently (see point 1 above)

By the way, I absolutely love the color "transparency" scale slider in the circular view. This really enables user to explore interactively. Thanks!

@jrobinso jrobinso merged commit 73d8821 into igvteam:master Jan 5, 2022
@jrobinso
Copy link
Contributor

jrobinso commented Jan 5, 2022

cheehongsg I've merged the PR. As this is not a released feature I merged it rather quickly so we can continue joint development on the same code base. These are really good additions for all users, especially the color coding by chromosome. Also thanks for the bug fixes.

I did have some questions though on the motivations for (2). The arcs should not be replaced when sending additional arcs for the same "track", at least that was not the intent. If that was happening it is a bug. If that is the motivation for including "range" I think it would be better to fix the bug. Perhaps you have other reasons though to want to treat distinct ranges as separate "tracks"? There's some conceptual advantage to keeping the concept of track 1-1 between the 2 views, especially when comparing multiple samples or a tumor/normal. I have a similar question about 2.1. We might need different configurable options for this.

@jrobinso
Copy link
Contributor

jrobinso commented Jan 5, 2022

@cheehongsg RE 3.2, this might be confusing, and the y-axis as drawn is incorrect. I think it might be better to have an explicit filter menu item and not overload the meaning of "data range" like this for interaction track only. If we keep the behavior we should do it consistently (i.e. for wig tracks also). I'm a bit conflicted, but I lean towards the explicit filter simply because data range has been used as it is for a very long time. Thoughts?

@cheehongsg
Copy link
Contributor Author

@jrobinso Thanks for the quick merge.

I did have some questions though on the motivations for (2). The arcs should not be replaced when sending additional arcs for the same "track", at least that was not the intent. If that was happening it is a bug. If that is the motivation for including "range" I think it would be better to fix the bug. Perhaps you have other reasons though to want to treat distinct ranges as separate "tracks"? There's some conceptual advantage to keeping the concept of track 1-1 between the 2 views, especially when comparing multiple samples or a tumor/normal. I have a similar question about 2.1. We might need different configurable options for this.

It is a mixture. Let me check if I can still reproduce the replacement issue with the latest code.

I do look at many different loci and build up the circular view a lot during exploratory phase. Having them as different tracks allow one to use the "Hide" button to permute rather than rebuilding from scratch. It also allow control at subset of cis-interactions. But, it does quickly overload the tracks control display.

I agree with your considerations and am open to other approaches.

@cheehongsg RE 3.2, this might be confusing, and the y-axis as drawn is incorrect. I think it might be better to have an explicit filter menu item and not overload the meaning of "data range" like this for interaction track only. If we keep the behavior we should do it consistently (i.e. for wig tracks also). I'm a bit conflicted, but I lean towards the explicit filter simply because data range has been used as it is for a very long time. Thoughts?

Is data range a visual range and/or a data (filter) range. It ended up all three interpretations are valid in current IGV. Visual range changes the axis. Data range subsets the data. The "combined" effect is what IGV has been showing.

A quick way to produce a correct output is to move the min y-axis value label along the y-axis to reflect the proper position.
But I am for explicitness to remove ambiguity. So, I second adding a "data filter?".

@jrobinso
Copy link
Contributor

@cheehongsg I just pushed a change that allows chords to be grouped by locus (locus at the time they are sent), or track. The grouping is done from circular-view and can be changed. I think the usage is pretty obvious but let me know if not.

We are getting closer to releasing this, releasing IGV with the circ view electron app took a lot longer than expected, but I am back on the javascript side now. Would you be interested in writing anything up on the Chia-Pet motivated changes, could be brief? If so I will link to it.

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.

None yet

2 participants