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

Get column names from BED tabix files and other utils for external jbrowse-plugin-gwas support #1630

Merged
merged 4 commits into from Feb 19, 2021

Conversation

cmdcolin
Copy link
Collaborator

This adds a "linear manhattan plot display"

Also makes it so that stats are available on the base adapter, so that means that all the different adapters types that use some basic stats (which includes, SNPCoverageAdapter, BigWigAdapter, GCCoverageAdapter (#1624), and now BEDTabix for manhattan plot) all don't have to individually implement stats

This is similar to jbrowse 1 where the base seqfeature store has a local stats operation

Then it also adds ability for BedTabixAdapter to choose it's "score column" and read column names from the header of the bedtabix file if available

This allows us to choose the column named "neg_log_pvalue" in the summary_stats.txt.gz file as the score that is plotted with the manhattan plot from a tabix file prepared from summary_stats.txt.gz from here https://my.locuszoom.org/gwas/236887/

localhost_3000__config=test_data%2Fconfig_demo json session=local-ioGQhLLNk (1)
localhost_3000__config=test_data%2Fconfig_demo json session=local-ioGQhLLNk

Can display whole-genome-wide results and local results

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Jan 21, 2021
@codecov
Copy link

codecov bot commented Jan 21, 2021

Codecov Report

Merging #1630 (cd7be9b) into master (708bafa) will increase coverage by 0.01%.
The diff coverage is 84.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1630      +/-   ##
==========================================
+ Coverage   58.90%   58.92%   +0.01%     
==========================================
  Files         446      446              
  Lines       20467    20493      +26     
  Branches     4824     4834      +10     
==========================================
+ Hits        12057    12076      +19     
- Misses       8112     8119       +7     
  Partials      298      298              
Impacted Files Coverage Δ
plugins/bed/src/BedTabixAdapter/BedTabixAdapter.ts 88.23% <83.33%> (-4.79%) ⬇️
...gle/src/LinearWiggleDisplay/components/Tooltip.tsx 40.00% <100.00%> (ø)
plugins/wiggle/src/index.ts 100.00% <100.00%> (ø)
...inearGenomeView/components/RefNameAutocomplete.tsx 89.36% <0.00%> (-4.26%) ⬇️

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 708bafa...cd7be9b. Read the comment docs.

@cmdcolin
Copy link
Collaborator Author

There are some potential add ons to this that could include

  1. Making this a custom track type, otherwise it is part of featuretrack and runs into confusion trying to be solved by Add new menu items for show/hide feature labels, set max height, and set compact display mode #1394
  2. Adding some informative mouseover for the dots, might not be necessary to get the point across though

@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 Jan 21, 2021
@cmdcolin cmdcolin added this to To do in JBrowse team board via automation Jan 21, 2021
@cmdcolin cmdcolin moved this from To do to In progress in JBrowse team board Jan 21, 2021
@cmdcolin
Copy link
Collaborator Author

cmdcolin commented Jan 22, 2021

Random observation: the manhattan plot on the my.locuszoom.org page is rendered with SVG using binned data, so each X coordinate is a line and a circle at the top instead of millions of circles. Apparently, it's also server side rendered, so you don't see any accessing the data file and only a small amount of rendered svg is downloaded over the wire to view that plot. It's still relatively slow to view that same track whole genome in this branch but there are probably some optimizations, and it is also possible to zoom in on jbrowse 2 which is important...

@cmdcolin cmdcolin mentioned this pull request Jan 22, 2021
@cmdcolin cmdcolin changed the base branch from master to stats_base_adapter January 26, 2021 16:49
@cmdcolin
Copy link
Collaborator Author

Rebased onto #1640

Base automatically changed from stats_base_adapter to master January 26, 2021 22:42
@cmdcolin
Copy link
Collaborator Author

This PR sort of has a couple options to go forward

  1. This GWAS module splits off into an external plugin. This has challenges because the gwas plugin uses a fair amount of stuff from @jbrowse/plugin-wiggle. So, either it adds a NPM dependency to @jbrowse/plugin-wiggle, which could go stale, or we copy and paste the wiggle stuff into the external GWAS plugin so it is more self sufficient, or we figure out a way to share plugin resources with our reexports?
  2. We accept it as is and improve it in the core repo

@garrettjstevens
Copy link
Collaborator

or we figure out a way to share plugin resources with our reexports

You can do this with exports on the plugin class. There's an example of the GDC plugin using LGV plugin resources here: https://github.com/GMOD/jbrowse-plugin-gdc/blob/c6d89158a0b4fa955f42eb68da01c04cb68540e1/src/index.ts#L27-L30

@cmdcolin
Copy link
Collaborator Author

thanks, that is quite interesting

I found that if I tried to add a proper @jbrowse/plugin-wiggle to my devdeps I got some very weird errors even trying to run yarn

ed by node_modules/@gmod/binary-parser/dist/binary_parser.js, but could not be resolved – treating it as an external dependency
'fs' is imported by node_modules/canvas/index.js, but could not be resolved – treating it as an external dependency
'fs' is imported by node_modules/canvas/index.js, but could not be resolved – treating it as an external dependency
'zlib' is imported by node_modules/@gmod/bbi/dist/blockView.js, but could not be resolved – treating it as an external dependency
'zlib' is imported by node_modules/@gmod/bbi/dist/blockView.js, but could not be resolved – treating it as an external dependency
'fs' is imported by fs?commonjs-external, but could not be resolved – treating it as an external dependency
'zlib' is imported by zlib?commonjs-external, but could not be resolved – treating it as an external dependency
'vm' is imported by vm?commonjs-external, but could not be resolved – treating it as an external dependency
'fs' is imported by fs?commonjs-external, but could not be resolved – treating it as an external dependency
'zlib' is imported by zlib?commonjs-external, but could not be resolved – treating it as an external dependency
'vm' is imported by vm?commonjs-external, but could not be resolved – treating it as an external dError: Unexpected character '' (Note that you need plugins to import files that are not JavaScript)

at /home/cdiesh/src/jbrowse-plugin-gwas2/node_modules/canvas/build/Release/canvas.node:1:0

1: ELF>0W@@�@8@  p�p�$p����$$P�td�V�V�V

Q�tdGNU���SBw��u���K|����@
                            �@"�"
   ^
2: @D0� @Ā���� �@       (�@��D)�!�      ��@"��@H@҂���d  @Hp �(%
3: J` ! F���PL`����P
                    � ���

I may have to forego the typescripting on @jbrowse/plugin-wiggle and not have it in devdeps since that would be all that gains me

@cmdcolin
Copy link
Collaborator Author

I'll just modify this PR to erase core plugins/gwas from history
Seems a little esoteric and plugins can be a bit more agile sometimes

@garrettjstevens
Copy link
Collaborator

I found that if I tried to add a proper @jbrowse/plugin-wiggle to my devdeps I got some very weird errors even trying to run yarn

I get that error when doing a top-level import of @jbrowse/plugin-wiggle, but if I don't do a top-level import and use the ... as import('@jbrowse/plugin-wiggle') syntax it seems to be fine.

@cmdcolin
Copy link
Collaborator Author

awesome thanks for checking that out

@cmdcolin
Copy link
Collaborator Author

cmdcolin commented Feb 17, 2021

Alright, I ripped the gwas plugin out into an external plugin, thanks very much @garrettjstevens for the tips

If this remaining PR is merged then it would provide features that support to that plugin namely

  1. wiggle re-exports that @garrettjstevens mentioned
  2. BEDTabix header parsing
  3. choosing a "score" column from the BEDTabix header
  4. Actually using the column numbers specified in the tabix header for the BEDTabix instead of assuming 1,2,3 are chr,start,end

https://github.com/cmdcolin/jbrowse-plugin-gwas

@cmdcolin
Copy link
Collaborator Author

Tested and the plugin does work if using this branch

@cmdcolin cmdcolin changed the title Manhattan plot GWAS display Add BedTabixAdapter header parsing and other utils for external jbrowse-plugin-gwas support Feb 17, 2021
@cmdcolin cmdcolin marked this pull request as ready for review February 17, 2021 01:01
@cmdcolin cmdcolin changed the title Add BedTabixAdapter header parsing and other utils for external jbrowse-plugin-gwas support Get column names from BED tabix files and other utils for external jbrowse-plugin-gwas support Feb 19, 2021
@rbuels rbuels merged commit e3ad824 into master Feb 19, 2021
JBrowse team board automation moved this from In progress to Done Feb 19, 2021
@rbuels rbuels deleted the gwas branch February 19, 2021 20: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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants