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

New display type for drawing arcs #2534

Merged
merged 25 commits into from Dec 15, 2021
Merged

New display type for drawing arcs #2534

merged 25 commits into from Dec 15, 2021

Conversation

carolinebridge
Copy link
Contributor

@carolinebridge carolinebridge commented Nov 17, 2021

summary of changes

  • adds a new display type, with accompanying renderer for drawing arcs on feature track data
  • arcs are clickable, hoverable, have the ability to be coloured, and variant thickness based on specified features with jexl
  • rendered arcs have the following configuration:
    color: {
      type: 'color',
      description: 'the color of the arcs',
      defaultValue: 'darkblue',
    },
    thickness: {
      type: 'number',
      description: 'the thickness of the arcs',
      defaultValue: `jexl:logThickness(feature,'score')`,
    },
    label: {
      type: 'string',
      description: 'the label to appear at the apex of the arcs',
      defaultValue: `jexl:get(feature,'score')`,
      contextVariable: ['feature'],
    },
    caption: {
      type: 'string',
      description:
        'the caption to appear when hovering over any point on the arcs',
      defaultValue: `jexl:get(feature,'name')`,
      contextVariable: ['feature'],
    },

@carolinebridge carolinebridge added the enhancement New feature or request label Nov 17, 2021
@carolinebridge carolinebridge self-assigned this Nov 17, 2021
@cmdcolin
Copy link
Collaborator

may want to use a package.json that is similar to other plugins in the monorepo, the external plugin template may not work with the build system

@carolinebridge
Copy link
Contributor Author

may want to use a package.json that is similar to other plugins in the monorepo, the external plugin template may not work with the build system

forgot to push it...lemme know what changes further need to be made to it ?

@cmdcolin
Copy link
Collaborator

interested user looking for this on jb2 elsiklab/sashimiplot#10

@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #2534 (88eb919) into main (cbfcd97) will increase coverage by 0.05%.
The diff coverage is 67.69%.

❗ Current head 88eb919 differs from pull request most recent head b759efa. Consider uploading reports for the commit b759efa to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2534      +/-   ##
==========================================
+ Coverage   60.86%   60.92%   +0.05%     
==========================================
  Files         547      551       +4     
  Lines       25727    25792      +65     
  Branches     6078     6084       +6     
==========================================
+ Hits        15659    15714      +55     
- Misses       9743     9753      +10     
  Partials      325      325              
Impacted Files Coverage Δ
products/jbrowse-desktop/src/corePlugins.ts 100.00% <ø> (ø)
products/jbrowse-web/src/corePlugins.ts 100.00% <ø> (ø)
plugins/arc/src/LinearArcDisplay/model.tsx 26.66% <26.66%> (ø)
plugins/arc/src/ArcRenderer/ArcRendering.tsx 75.75% <75.75%> (ø)
plugins/arc/src/index.ts 83.33% <83.33%> (ø)
packages/core/util/jexl.ts 68.42% <100.00%> (+0.56%) ⬆️
plugins/arc/src/LinearArcDisplay/configSchema.tsx 100.00% <100.00%> (ø)
...aseLinearDisplay/models/BaseLinearDisplayModel.tsx 83.26% <0.00%> (+0.41%) ⬆️
packages/core/util/layouts/GranularRectLayout.ts 87.87% <0.00%> (+0.43%) ⬆️
packages/core/PluginManager.ts 93.38% <0.00%> (+0.82%) ⬆️
... and 4 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 cbfcd97...b759efa. Read the comment docs.

@cmdcolin
Copy link
Collaborator

we may want to review whether we want to do inter-region arcs with this. it would be useful for things like connecting structural variants probably

@cmdcolin
Copy link
Collaborator

example: connecting an arc from chr1 to chr2

@cmdcolin
Copy link
Collaborator

that is not currently possible on this branch AFAIK

@carolinebridge
Copy link
Contributor Author

that is not currently possible on this branch AFAIK

correct, would you like it to be done as a part of this PR?

@cmdcolin
Copy link
Collaborator

another thing that could be considered is: automatically generating an arc track from an RNA-seq data BAM file (for getting an automatic sashimi plot, this can be done for sashimiplot in jb1)

another application of arcs to structural variants is having an arc for each pair of reads, this was done in jb1 and would be great in jb2, but would especially be nice to have with arcs that can go across mutliple displayed regions

https://jbrowse.org/code/JBrowse-1.16.11/?loc=ctgA%3A56..5073&tracks=DNA%2CTranscript%2Cvolvox_microarray_bw_density%2Cvolvox_microarray_bw_xyplot%2Cvolvox-sorted-vcf%2Cvolvox-sorted_bam_coverage%2Cvolvox-sv-cram&data=sample_data%2Fjson%2Fvolvox&tracklist=1&highlight=&nav=1&overview=1

@cmdcolin
Copy link
Collaborator

correct, would you like it to be done as a part of this PR?

I guess it would be interesting if it could be considered...if needed it could be done separately as a separate renderer or display but i think having it done once and for all would be quite cool. also, see the jbrowse 1 link where it involves many many arcs...could possibly slow down with svg

@cmdcolin
Copy link
Collaborator

(have to go to "view as arcs" in the track menu to get the arcs)

r1

@carolinebridge
Copy link
Contributor Author

carolinebridge commented Nov 24, 2021

notes from meeting:

  • register to core feature track
  • register another display using arcs to alignments track
  • add demo to volvox ;; dbsuper

@rbuels rbuels changed the title migrating arc plugin New display type for drawing arcs Nov 24, 2021
@carolinebridge
Copy link
Contributor Author

This PR includes adding the LinearArcDisplay type to the FeatureTrack. While this allows for the arc display to be accessible to users, using it feels a little awkward. Given that a FeatureTrack isn't 'meant' to be viewed as an arc, it just doesn't make sense to allow a user to toggle these displays, and having the ability to do such a thing so readily may confuse them. To this end, we may want to re-evaluate how we expose toggling track display types to the user. This may involve an extra layer of UI (e.g., a display selector), aliases for the 'internal' names for display types, and/or tooltips.

I feel that adding one, or a combination, of these suggestions would make it easier to add displays like this to tracks in the future, while maintaining a clear intention to the user and communicating how tools are meant to be used without referring to docs or external resources.

That being said, because there isn't a satisfactory solution at the moment for this, we still feel like the PR should go through to make this utility available to users while we consider how to better expose display types to users.

@carolinebridge
Copy link
Contributor Author

#2550 potential solution to the question of clarity in terms of changing the displays mentioned above (i.e. provides an alias and a description of the displays)

@carolinebridge
Copy link
Contributor Author

discuss in meeting : potentially add a dialog for changing display that provides the description clearly to the user without the concern for a description being too long for the track menu

@cmdcolin
Copy link
Collaborator

I made a small change that makes the arcs have slightly different heights by making the arc height proportional to the Math.log of feature width (end-start)

diff --git a/packages/core/util/jexl.ts b/packages/core/util/jexl.ts
index 812f337c2..580eafbc2 100644
--- a/packages/core/util/jexl.ts
+++ b/packages/core/util/jexl.ts
@@ -42,6 +42,7 @@ export default function (/* config?: any*/): JexlNonBuildable {
   j.addFunction('floor', Math.floor)
   j.addFunction('round', Math.round)
   j.addFunction('abs', Math.abs)
+  j.addFunction('log10', Math.log10)
   j.addFunction('parseInt', Number.parseInt)
   j.addFunction('parseFloat', Number.parseFloat)
 
diff --git a/plugins/arc/src/ArcRenderer/ArcRendering.tsx b/plugins/arc/src/ArcRenderer/ArcRendering.tsx
index c4490ab70..d73af968e 100644
--- a/plugins/arc/src/ArcRenderer/ArcRendering.tsx
+++ b/plugins/arc/src/ArcRenderer/ArcRendering.tsx
@@ -47,6 +47,7 @@ function ArcRendering(props: any) {
     const label = readConfObject(config, 'label', { feature })
     const caption = readConfObject(config, 'caption', { feature })
     const strokeWidth = readConfObject(config, 'thickness', { feature })
+    const height = readConfObject(config, 'height', { feature }) || 100
     const ref = React.createRef<SVGPathElement>()
     const tooltipWidth = 20 + measureText(caption?.toString())
 
@@ -54,7 +55,7 @@ function ArcRendering(props: any) {
       <g key={id} onClick={e => onClick(e, featureId)}>
         <path
           id={id}
-          d={`M ${left} 0 C ${left} 100, ${right} 100, ${right} 0`}
+          d={`M ${left} 0 C ${left} ${height}, ${right} ${height}, ${right} 0`}
           stroke={stroke}
           strokeWidth={strokeWidth}
           fill="transparent"
diff --git a/plugins/arc/src/ArcRenderer/configSchema.tsx b/plugins/arc/src/ArcRenderer/configSchema.tsx
index 14b5b74db..3e0bdd72d 100644
--- a/plugins/arc/src/ArcRenderer/configSchema.tsx
+++ b/plugins/arc/src/ArcRenderer/configSchema.tsx
@@ -19,6 +19,11 @@ export default ConfigurationSchema(
       defaultValue: `jexl:get(feature,'score')`,
       contextVariable: ['feature'],
     },
+    height: {
+      type: 'number',
+      description: 'the height of the arcs',
+      defaultValue: `jexl:log10(get(feature,'end')-get(feature,'start'))*50`,
+    },
     caption: {

before

localhost_3000__config=test_data%2Fvolvox%2Fconfig json session=local-cNGIYVDd9 (1)

after

localhost_3000__config=test_data%2Fvolvox%2Fconfig json session=local-cNGIYVDd9

that makes the larger arcs maybe more easily clickable in some cases

@carolinebridge
Copy link
Contributor Author

I made a small change that makes the arcs have slightly different heights by making the arc height proportional to the Math.log of feature width (end-start)

did you end up pushing this?

@cmdcolin
Copy link
Collaborator

did not push that change yet. was a little bit wondering if the label height (78px thing) should be parameterized if the height becomes configurable

@carolinebridge
Copy link
Contributor Author

so the equation above didn't end up working because our curves are actually cubic (i thought they were quadratic)

slight modification to the calculation, which is actually a lot more straight forward

const t = 0.5

const textYCoord =
      (1 - t) * (1 - t) * (1 - t) * 0 +
      3 * ((1 - t) * (1 - t)) * (t * height) +
      3 * (1 - t) * (t * t) * height +
      t * t * t * 0

this formula is plucked from https://en.wikipedia.org/wiki/B%C3%A9zier_curve#Cubic_B%C3%A9zier_curves

t is always going to be 0.5 because that's where we want on the bezier curve

on the component i've got y={textYCoord+3} because this calculation gets the height of the vertex on the curve, and I want to place the text centered in the arc. if i omit the +3 the text sits on top of the arc, which doesn't look as good.

demo:

image

@cmdcolin
Copy link
Collaborator

awesome :) can you add the wiki link next to the formula, then we can merge!

@cmdcolin cmdcolin merged commit 3dabffa into main Dec 15, 2021
@cmdcolin
Copy link
Collaborator

it's beautiful :')

@cmdcolin
Copy link
Collaborator

cmdcolin commented Dec 15, 2021

note: making it capable of cross-region arcs would be very good for SV visualization also #2569 if you want to keep that on the burner

@cmdcolin cmdcolin deleted the add-arc-plugin branch December 16, 2021 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants