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

Transcript glyphs #458

Merged
merged 30 commits into from
Aug 16, 2019
Merged

Transcript glyphs #458

merged 30 commits into from
Aug 16, 2019

Conversation

garrettjstevens
Copy link
Collaborator

This adds a ProecessedTranscript glyph, as well as some other glyphs that are used to make it (Chevron, Segments, and Subfeature). Here's a side-by-side of Jbrowse 1 & 2. More work could be done in the future to port more of the config information from JB1 to JB2 (like colors, etc.), but the main information is there.

image

resolves #240

@garrettjstevens garrettjstevens self-assigned this Aug 14, 2019
@cmdcolin
Copy link
Collaborator

I'm not a big fan of the little angle things... I think it's visually noisy and makes it actually confusing where the true boundary of a feature is.

@garrettjstevens
Copy link
Collaborator Author

I like having directionality indicated by the glyph, I think it's informative, but I agree it does need a bit of work. I though about making only one end pointed, like this:

image

The only thing I don't like about that is if you're zoomed in to the flat end, you can't tell if it's just a box or has direction.

@garrettjstevens
Copy link
Collaborator Author

I'd like to keep this PR focused on the SVG glyph architecture and not specific glyphs themselves, so I created #460 to keep discussions on glyphs.

@codecov
Copy link

codecov bot commented Aug 14, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@00b0596). Click here to learn what that means.
The diff coverage is 62.46%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #458   +/-   ##
========================================
  Coverage          ?   64.1%           
========================================
  Files             ?     238           
  Lines             ?    7753           
  Branches          ?    1785           
========================================
  Hits              ?    4970           
  Misses            ?    2557           
  Partials          ?     226
Impacted Files Coverage Δ
packages/core/components/ResizeHandleHorizontal.js 36.66% <ø> (ø)
...leElementTypes/renderers/ServerSideRendererType.js 100% <ø> (ø)
...iew/src/BasicTrack/util/serverSideRenderedBlock.js 91.42% <ø> (ø)
packages/protein-widget/src/Viewer.js 71.66% <ø> (ø)
packages/jbrowse-web/src/rpc.worker.js 45.16% <ø> (ø)
packages/core/BaseAdapter.ts 89.47% <ø> (ø)
packages/core/rpc/RpcManager.js 91.89% <ø> (ø)
packages/core/util/index.ts 59.68% <ø> (ø)
...w/src/CircularView/models/viewportVisibleRegion.js 84.09% <ø> (ø)
...ckages/jbrowse1/src/JBrowse1Connection/jb1ToJb2.ts 0% <0%> (ø)
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00b0596...4d93587. Read the comment docs.

@cmdcolin
Copy link
Collaborator

Currently the human Gencode 19 track is failing with a TypeError

I can see certain things like hasSubSub and the Subfeatures glyph and I don't really understand the purpose of these?

We need to standardize a strand representation so that we do not run into having to check for -1,1,+,and - in all cases, just one of the other.

Sort of concerned about mRNA and transcript both being ProcessedTranscript by default.

@garrettjstevens
Copy link
Collaborator Author

I'm fine with standardizing, it looks like everything so far uses 1 and -1. Should we standardize on that?

Also, I was using JB1 as a guide in determining glyph types, but there could be something I'm misunderstanding:
https://github.com/GMOD/jbrowse/blob/c10e55ba2924be0f72fde0b36c57a9881ddf3038/src/JBrowse/View/Track/CanvasFeatures.js#L212-L220.

The Subfeatures glyph lays out its child glyphs vertically. This glyph gets used whenever one of the subfeatures has subfeatures itself.

I can see the Gencode v19 error, I'll see what's going on there. I also realized that reversing does not work properly, so that will definitely need to get fixed, too.

@garrettjstevens
Copy link
Collaborator Author

The Gencode track doesn't error now, and the glyphs now display properly when the view is reversed.

bpPerPx,
horizontallyFlipped,
config,
}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

A function on the function? Is this a thing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With the original Box glyph, layOut was a static method on the class component. When I refactored to hooks, this seemed to be the equivalent way to do it (similarly to how propTypes can be a static property of a class component or defined on the function).

@rbuels
Copy link
Contributor

rbuels commented Aug 15, 2019

Could you add a little explanation of the glyph architecture?

@cmdcolin
Copy link
Collaborator

If possible it would be good to have a rendering of the non-coding features that isn't just a line, currently it is just a line for the non-coding features I think on the Gencode test data

@garrettjstevens
Copy link
Collaborator Author

@rbuels I've added a readme

@cmdcolin I added a little arrow to the end so it's not just a line, similar to JB1.

image
image

@cmdcolin
Copy link
Collaborator

Some of the features like ENSG00000227169.2 have feature.get('type') undefined?

MODEL.views[0].setNewView(5.377899045020464,857364.7635717909)

@garrettjstevens
Copy link
Collaborator Author

I'm not sure what you mean. That feature's type is 'gene'.

@cmdcolin
Copy link
Collaborator

Oh sorry I was looking at it console.log(feature.get('name')) on it and thought I was logging type..

But I guess I am wondering why does that feature not have exons in the displayed glyph?

test

@garrettjstevens
Copy link
Collaborator Author

I thought exons would be part of #241. The current behavior does seem consistent with JB1 under certain configurations, though:

image

My JB1 track is configured as

[tracks.TestTrack]
category=Other
key=Gencode v19
label=ATestLabel
storeClass=JBrowse/Store/SeqFeature/NCList
type=JBrowse/View/Track/CanvasFeatures
transcriptType=transcript
urlTemplate=http://s3.amazonaws.com/jbrowse.org/genomes/hg19/gencode/{refseq}/trackData.json

@cmdcolin
Copy link
Collaborator

Ok so it filters so it only gets CDS and utr subfeatures and plots those and this feature only has exon subfeatures

@garrettjstevens garrettjstevens merged commit 0fc6b6f into master Aug 16, 2019
@garrettjstevens garrettjstevens deleted the transcript_glyphs branch August 16, 2019 20:10
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.

transcript SVG feature glyph
3 participants