Add sequence track for both read and reference genome in the "Linear read vs ref" comparison

[cmdcolin]

This adds a sequence track to read vs ref comparisons, which I thought could help detailed investigations (like seeing a repeat expansion, or similar)

Note 1: This may need qualification because it can only be done on the primary alignment, which could be obtained via redispatching or just made a qualification of this
Note 2: Get sequence on this sequence track produces error "failed to find assembly..." because the "read assembly" is not formally added to the assembly manager, which could be something to reconsider possibly
Note 3: Sort of similar 2, but this was sort of difficult to debug because assemblyManager produced an infinite wait on not found assemblyName instead of any error

[cmdcolin]

Now automatically opens both the reference genome's sequence track and the "read sequence" track, which was quite helpful for properly visualizing the comparison...this also uncovered a bug in the read vs ref (ebcbfa6) that was missed before...quite helpful to see the underlying sequence

[codecov]

Codecov Report

Merging #1699 (f65b2b0) into master (7186f1b) will decrease coverage by 0.16%.
The diff coverage is 40.49%.

@@            Coverage Diff             @@
##           master    #1699      +/-   ##
==========================================
- Coverage   59.18%   59.02%   -0.17%     
==========================================
  Files         446      446              
  Lines       20480    20535      +55     
  Branches     4832     4851      +19     
==========================================
- Hits        12121    12120       -1     
- Misses       8059     8115      +56     
  Partials      300      300              

Impacted Files	Coverage Δ
...omparative-view/src/LinearComparativeView/model.ts	9.47% <0.00%> (-0.53%)	⬇️
plugins/linear-comparative-view/src/index.tsx	27.22% <0.00%> (-7.92%)	⬇️
...play/components/ServerSideRenderedBlockContent.tsx	91.66% <ø> (-0.65%)	⬇️
...quenceRenderer/components/DivSequenceRendering.tsx	94.59% <ø> (-0.33%)	⬇️
products/jbrowse-web/src/sessionModelFactory.ts	61.66% <22.22%> (-1.35%)	⬇️
plugins/dotplot-view/src/index.ts	38.39% <60.00%> (+1.12%)	⬆️
...aseLinearDisplay/models/serverSideRenderedBlock.ts	95.65% <80.00%> (-0.25%)	⬇️
packages/core/util/index.ts	82.20% <90.90%> (+0.32%)	⬆️
packages/core/assemblyManager/assemblyManager.ts	59.42% <100.00%> (+0.59%)	⬆️
packages/core/rpc/coreRpcMethods.ts	86.36% <100.00%> (+0.64%)	⬆️
... and 8 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 7186f1b...f65b2b0. Read the comment docs.

[elliothershberg]

Wow this is a considerable improvement I think

[cmdcolin]

@elliothershberg thanks

i think there is a real need to visualize insertions. even this current strategy isn't that great since it is only 1-read-at-a-time but this information about insertions is otherwise totally hidden from users without being able to see the sequence

[cmdcolin]

Adding sequence track of course can help things besides insertions but insertions can really benefit...can possibly see things like (tri-nucleotide)repeat expansion

[cmdcolin]

The concerns mentioned in the original post have been solved using some various techniques

It continues to elaborate the concept of the "view assembly" which exists outside of the assembly manager

So now we have these things:

a) the code does a redispatch request to find the primary alignment if the user had selected the supplementary part of a split alignment
b) user can get sequence on the "read" in the read vs ref view
c) the code bypasses ref renaming/assembly manager stuff through use of a config flag noAssemblyManager

[cmdcolin]

I marked this as ready for review. I think probably the most notable review point would be the continued usage of viewAssemblyConfigs instead of assemblyManager

This is a method for the "read vs ref" to have a "read assembly"

Any concerns about this?

[cmdcolin]

Possible alternative approach to the viewAssemblyConfigs:

• add proper session assemblies
• then the session assembly is properly added to assemblyManager
• have autorun that automatically removes the read assembly from the sessionAssembly (this way instead of the viewAssemblyConfig being removed when view goes away, something else automatically removes it from session assembly)

xref #1445

[cmdcolin]

After doing the rubber duck debugging in the above post and considering the options that I listed kind of were not going anywhere, I went "full sessionAssemblies" and so now the read vs ref comparison performs session.addAssembly and when the view is destroyed it performs a session.removeAssembly.

[cmdcolin]

Also removed the notion of the assemblyManager.addAssembly allowing SnapshotOrInstance (changed to Instance)

[garrettjstevens]

In general I've avoided having the assemblyManager have a handle on the session to tried and keep it a bit more independent of an entity, but I guess that not totally necessary. As long as it handles switching from a session with a session assembly to one without and vice versa, it should be fine.

Another option could be to use an ephemeral assembly config, xref #1647

[cmdcolin]

I think I actually want the assembly to be persisted and not ephemeral though, so that a read vs ref can be shared

[rbuels]

i think use something more specific than '-temp' in case other views are doing the same thing, so you don't delete their temp assemblies.

actually probably need to use this view's unique id to establish ownership so you don't delete temp assemblies from other instances of LinearComparativeView

[cmdcolin]

the -temp concept is created before the view is created so no view.id available, also only looks in the view's assemblies so the "damage" should be minimal

[rbuels]

good to merge when those little comments are addressed

[cmdcolin]

The FromConfigSequenceAdapter now makes no attempt to combine multiple features from the config when doing requests. Rather than try to do this somewhat complicated behavior it just tries to do it in a more simple manner....this then returns a single feature that has the feat.get('seq') match the region being requested @elliothershberg @garrettjstevens

[cmdcolin]

if that behavior is needed we can add it and refine tests for it, i think the existing code was incomplete for that case