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

Add "Get sequence" action to LGV rubber-band #1588

Merged
merged 77 commits into from
Jan 15, 2021
Merged

Conversation

teresam856
Copy link
Contributor

@teresam856 teresam856 commented Dec 24, 2020

Issue 898

Get the reference sequence for the rubber-banded area.

Created get sequence dialog that opens when a section in the LGV rubber-band is selected.

TODO:
Pops up a modal dialog that has:

  • button to copy to clipboard
  • filename text field (with good default name)
  • Regions selected contains out of bound coord
  • Can select all regions
  • Can select entire region
  • Can select in between regions
  • Can select multiple displayed Regions

Error Handling

  • Disabled if there is no sequence adapter for the assembly in question
  • Disabled (grayed out) if more than 500MB
  • copy to clipboard (turned off if no clipboard support)
  • textarea with the exported sequence in FASTA format, textarea is not displayed if more than 100KB of sequence

Tests

  • tests for dialog opening/closing
  • tests get selectedRegions
  • test format and size of file

@teresam856 teresam856 self-assigned this Dec 24, 2020
@teresam856 teresam856 marked this pull request as draft December 24, 2020 00:17
@codecov
Copy link

codecov bot commented Jan 2, 2021

Codecov Report

Merging #1588 (111b2ed) into master (2c20e69) will increase coverage by 0.13%.
The diff coverage is 73.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1588      +/-   ##
==========================================
+ Coverage   58.98%   59.12%   +0.13%     
==========================================
  Files         439      441       +2     
  Lines       20108    20250     +142     
  Branches     4609     4654      +45     
==========================================
+ Hits        11861    11973     +112     
- Misses       7951     7978      +27     
- Partials      296      299       +3     
Impacted Files Coverage Δ
...c/LinearGenomeView/components/LinearGenomeView.tsx 78.78% <60.00%> (-3.36%) ⬇️
...src/LinearGenomeView/components/SequenceDialog.tsx 63.41% <63.41%> (ø)
packages/core/util/Base1DViewModel.ts 77.20% <83.33%> (+5.43%) ⬆️
...iew/src/LinearGenomeView/components/RubberBand.tsx 85.34% <83.33%> (-0.52%) ⬇️
packages/core/rpc/coreRpcMethods.ts 85.24% <90.00%> (+0.93%) ⬆️
packages/core/util/formatFastaStrings.ts 100.00% <100.00%> (ø)
...s/linear-genome-view/src/LinearGenomeView/index.ts 80.61% <100.00%> (+0.39%) ⬆️
... and 1 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 2c20e69...111b2ed. Read the comment docs.

@teresam856
Copy link
Contributor Author

Untitled.2.mov

@rbuels
Copy link
Contributor

rbuels commented Jan 4, 2021

@scottcain look at that video ;-)

@elliothershberg
Copy link
Member

🤩 Looks awesome! How tricky would it be to also support getting the locstring instead of sequence? Would be useful for certain use cases I think.

@cmdcolin
Copy link
Collaborator

cmdcolin commented Jan 4, 2021

I would maybe suggest that the "Get sequence" by default grabs FASTA, and that each displayed region is a section of FASTA with the locstring in the header so it might look like

>ctgA:1-100
ACGTACGTACGATCGATCGACGATCGATCGATCGATCGATGCTAGCTA

>ctgA:201-300
ACGTACGTACGATCGATCGACGATCGATCGATCGATCGATGCTAGCTA

@garrettjstevens garrettjstevens added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Jan 6, 2021
@cmdcolin
Copy link
Collaborator

cmdcolin commented Jan 14, 2021

This is a bit of a weird idea but I thought maybe we could simplify the current implementation of getSelectedRegions using dynamicBlocks, since dynamicBlocks basically gives us the calculated "visible regions"...so I thought maybe we could use it to get selected regions too

I created a "simulated" Base1DView from a snapshot of the LinearGenomeView, and then simulate zoomToDisplayedRegions on it, and then get simulatedView.dynamicBlocks

https://github.com/GMOD/jbrowse-components/tree/898_get_sequence_mod

Random other thing but the branch also uses a monospace font for the textarea

@cmdcolin cmdcolin marked this pull request as ready for review January 14, 2021 22:20
@cmdcolin cmdcolin changed the title get sequence from LGV rubber-band Add "Get sequence" action to LGV rubber-band Jan 15, 2021
@cmdcolin cmdcolin merged commit cf8138d into master Jan 15, 2021
@cmdcolin
Copy link
Collaborator

went ahead and merged ^_^ if there are any follow ups maybe we can do new issues but afaict it works good

@cmdcolin cmdcolin deleted the 898_get_sequence branch January 15, 2021 17:58
@teresam856
Copy link
Contributor Author

Jbrowse2_get_sequence.mov

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.

5 participants