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

Coloring BAM reads by strand with mismatch highlights #72

Merged
merged 28 commits into from
Jul 18, 2014

Conversation

ymen
Copy link

@ymen ymen commented Jul 17, 2014

Addresses: #67

Sorry for taking such a long time for the PR, and definitely let us know your opinions and suggestions. We would love to work with you to improve implementations and design choices, and make sure the changes align to the overall roadmap of Dalliance!

Changes

This PR implements coloring of BAM reads by strand, rendered using _SEQUENCE glyphs.

The option to color BAM reads is turned on by the option Highlight mismatches & strands, piggybacking on the previous option Color mismatches. This option is currently not turned on by default, but I believe it might be reasonable to make this turned on by default for Bam files.

Specific changes as follows:

  • Changes to edit tier panel:
    Color mismatches renamed as Highlight mismatches & strands.

    When checked, adds 2 color selector rows for selecting colors of plus strand (default: lightsalmon) and the minus strand (default: lightskyblue)
    image

  • Changes to glyph rendering (when color strand option selected)

    At close up view (defined as scale > 8), draw rectangle for each base: bases that agree with ref bases are colored using strand colors, as selected in the tier-edit panel. Non-ref bases' rects are colored based on their base colors (defined in the object baseColors in cbrowser.js) and labelled with black text. Opacity is adjusted inverse to the read quality score, as previously.

image

At high zoom without closeup (defined as zoom=high & scale < 8), reads are first drawn as a solid rectangle based on strand direction with height corresponding to half of the tier height. Non-ref bases are drawn over with full height.
From working with the browser, we have found this to 1) improve visibility of non-ref bases and 2) reduce rendering time, especially for high depth (>1000) sequencing results. Ref bases are fully opaque and not adjusted based on qual score.

image

  • Changes to toSVG rendering (when color strand option selected)

    SVG rendering always draw each base individually, at full height, with opacity adjusted to qual score, since rendering time is probably not a huge consideration in this case.

image

T: 'red'
G: 'orange',
T: 'red',
'-' : 'hotPink' // deletion
Copy link
Author

Choose a reason for hiding this comment

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

Perhaps there could be an option to customize these colors.

Also, the '-' character denoting deletion is currently just hardcoded here, which is probably not ideal.

@dasmoth
Copy link
Owner

dasmoth commented Jul 17, 2014

Thanks very much for this contribution.

A few minor things:

  • There seems to be a typo of "hotPink" vs "hotpink" in the colour names
  • Is there any particular reason for changing the default colour for "G"s from black to orange? Black seems to be the common choice in chromatogram-viewing software. In the longer term, I agree that these ought to be editable!
  • I don't really understand why the "colors" object needs to be constructed and passed to the SequenceGlyph constructor -- this seems like unnecessary overhead. You already know the orientation of the read at the time when the SequenceGlyph is constructed, so you can choose the plus or minus colour as appropriate and pass that in directly. Passing the baseColors object to the SequenceGlyph is fine (since there hopefully won't be too many of those objects in the process!) but I'd just add an extra argument to the SequenceGlyph constructor?

Are you happy to make those changes, or shall I?

Slightly longer-term things I think we should be considering (but let's not hold up this PR, which is clearly useful as is):

  • It would be nice if it was possible to colour other types of feature by strand using the same UI.

  • Interaction with the rest of the stylesheet system: it's actually possible to make style rules which filter by orientation, e.g:

                <STYLESHEET>
                   <CATEGORY id='default'>
                       <TYPE id='foo' orientation='+'>
                         <BOX>
                           <BGCOLOR>red</BGCOLOR>
                        </BOX>
                       </TYPE>
                       <TYPE id='foo' orientation='-'>
                         <BOX>
                           <BGCOLOR>blue</BGCOLOR>
                        </BOX>
                       </TYPE>
                   </CATEGORY>
               </STYLESHEET>
    

I've just realized that this Dalliance-specific stylesheet extension never made it into the docs (my bad, will fix shortly). Doing things this way is more general, at the expense of making it a little harder to hook everything up in the track editor. Just something to consider in the future.

@mdrasmus
Copy link

Is there any particular reason for changing the default colour for "G"s from black to orange?

@ymen correct me if I'm wrong, but I believe we made this change because using black for the characters was better for readability.
https://github.com/dasmoth/dalliance/pull/72/files#diff-da2a15b516d657a0793c92a72ded3428R1105

@dasmoth
Copy link
Owner

dasmoth commented Jul 17, 2014

It's this change I was talking about:

https://github.com/dasmoth/dalliance/pull/72/files#diff-0d966b7245df74fbb0a38172982c9717L107

On Thu, Jul 17, 2014 at 4:46 PM, Matt Rasmussen notifications@github.com
wrote:

Is there any particular reason for changing the default colour for "G"s
from black to orange?

@ymen https://github.com/ymen correct me if I'm wrong, but I believe we
made this change because using black for the characters was better for
readability.

https://github.com/dasmoth/dalliance/pull/72/files#diff-da2a15b516d657a0793c92a72ded3428R1105


Reply to this email directly or view it on GitHub
#72 (comment).

@ymen
Copy link
Author

ymen commented Jul 17, 2014

Thanks Thomas for your feedback. I'll make the changes and push it back to you later today.

Is there any particular reason for changing the default colour for "G"s from black to orange? Black seems to be the common choice in chromatogram-viewing software.

I was following the colours from IGV, but this is pretty much arbitrary. Another consideration was that we had to change the font color for G if the base fill color for G is set to black, which was somewhat inconsistent visually. But if you think black for G works better I can just change it back, and label on it with white characters.

I'd just add an extra argument to the SequenceGlyph constructor?

Sounds good, I'll make the adjustment!

@ymen
Copy link
Author

ymen commented Jul 17, 2014

Pushed the correction for the typo and adjusted the SequenceGlyph constructor as suggested.

If you prefer the original base color for G, I'd be happy to make that change as well.

dasmoth added a commit that referenced this pull request Jul 18, 2014
Coloring BAM reads by strand with mismatch highlights
@dasmoth dasmoth merged commit caec755 into dasmoth:master Jul 18, 2014
@ymen ymen deleted the opensource_color_strands branch September 2, 2014 22:37
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

3 participants