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

Allow local files on the users computer to be opened as tracks in jbrowse-web #1975

Merged
merged 12 commits into from
May 19, 2021

Conversation

cmdcolin
Copy link
Collaborator

This is an early result but I had an interest in allowing the ability to upload local files as blob files in jbrowse web. This is a longstanding issue #88

The basic methodology is this

  1. realize that you can't store a file blob in a mobx-state-tree state, and our other file handles are mobx-state-tree's so then go to step 1
  2. create a mapping of a blobId (something I made up) to the actual file blob
  3. make the BlobLocation mst object store this blobId
  4. making openLocation read the map to access the file blob given the BlobLocation's blobId

In order to make this work on the webworker, we have to send the map of blobId to the blob over the rpc. This PR sends the entire map instead of trying to select a single blob off the map for example for smaller payload.

Fixes #88

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label May 14, 2021
@cmdcolin cmdcolin added enhancement New feature or request and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels May 14, 2021
@rbuels
Copy link
Contributor

rbuels commented May 14, 2021 via email

@cmdcolin
Copy link
Collaborator Author

There is just a failure to open the track in that case, it just says the blobId was not found. We could likely make some more special logic though e.g. our concept to prompt user to reopen those files

@rbuels
Copy link
Contributor

rbuels commented May 14, 2021

that's probably ok, as long as the error message is easy-ish to understand

@cmdcolin cmdcolin force-pushed the allow_local_file_blobs_jbweb branch from 45224cc to f5d75e6 Compare May 17, 2021 01:07
@codecov
Copy link

codecov bot commented May 17, 2021

Codecov Report

Merging #1975 (d34eb9a) into main (25cad08) will increase coverage by 0.01%.
The diff coverage is 46.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1975      +/-   ##
==========================================
+ Coverage   61.36%   61.37%   +0.01%     
==========================================
  Files         470      470              
  Lines       22210    22243      +33     
  Branches     5099     5113      +14     
==========================================
+ Hits        13629    13652      +23     
- Misses       8306     8316      +10     
  Partials      275      275              
Impacted Files Coverage Δ
packages/core/util/io/index.ts 51.61% <0.00%> (-5.53%) ⬇️
packages/core/util/types/index.ts 62.96% <ø> (ø)
...ent/src/AddTrackWidget/components/ConfirmTrack.tsx 2.77% <0.00%> (+0.50%) ⬆️
...lot-view/src/DotplotView/components/ImportForm.tsx 76.92% <ø> (ø)
.../legacy-jbrowse/src/JBrowse1Connection/jb1ToJb2.ts 0.00% <0.00%> (ø)
...ew/src/LinearSyntenyView/components/ImportForm.tsx 0.00% <ø> (ø)
packages/core/ui/FileSelector.tsx 60.00% <35.00%> (-8.63%) ⬇️
packages/core/rpc/BaseRpcDriver.ts 85.14% <50.00%> (+5.35%) ⬆️
...t/src/AddTrackWidget/components/AddTrackWidget.tsx 50.00% <50.00%> (-1.22%) ⬇️
packages/core/util/tracks.ts 61.68% <54.83%> (-2.37%) ⬇️
... and 7 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 25cad08...d34eb9a. Read the comment docs.

@cmdcolin
Copy link
Collaborator Author

Now gives some clearer text about how an end user might reload their files

localhost_3000__config=test_data%2Fvolvox%2Fconfig json session=local-goefosl-M

As far as reactivity, for alignmentstrack the pileup automatically redraws after loading the bam and bai, but the snpcoverage needs to manually hit reload. not exactly sure why

@cmdcolin
Copy link
Collaborator Author

@garrettjstevens do you think this method of adding blobMap at the super-base-level of RpcMethodType is ok?

@rbuels
Copy link
Contributor

rbuels commented May 19, 2021

looks good to me, merge if you think so too @garrettjstevens

@cmdcolin
Copy link
Collaborator Author

works on both firefox and midori (basically safari on linux) for me

Copy link
Collaborator

@garrettjstevens garrettjstevens left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@garrettjstevens garrettjstevens merged commit c387310 into main May 19, 2021
@cmdcolin cmdcolin deleted the allow_local_file_blobs_jbweb branch May 21, 2021 20:25
@cmdcolin cmdcolin changed the title Allow local file blobs jbweb Allow local files on the users computer to be opened as tracks in jbrowse-web May 21, 2021
@garrettjstevens
Copy link
Collaborator

For reference, screenshot of selector in URL and File states:

image

@cmdcolin
Copy link
Collaborator Author

cmdcolin commented May 24, 2021

this screenshot* may have been taken before a988ffe

@garrettjstevens
Copy link
Collaborator

garrettjstevens commented May 24, 2021

Whoops, I thought I had pulled the latest changes. Updated screenshot:

image

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.

Add ability to open local files in jbrowse-web
3 participants