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

Fix tabix histogram for CanvasFeatuers #956

Merged
merged 14 commits into from
Feb 6, 2018

Conversation

nathandunn
Copy link
Contributor

@nathandunn nathandunn commented Jan 18, 2018

Fixes #935

  • add volvox example with BigWig if not there

  • add volvox exmple without BigWig

  • adjust feature density to sane default, confirm adjustment

  • allows current use of supplying alternate store such as BigWig

  • Provides automated feature density for GFF3Tabix versus CanvasFeatures

    • implement getRegionFeatureDensities to get histograms
    • auto calculate scoreMax, max
    • implement getGlobal and getRegion?
  • provides reasonably similar densities to HTMLFeatures (or at least sane defaults)

@ghost ghost assigned nathandunn Jan 18, 2018
@ghost ghost added the in progress currently being worked on label Jan 18, 2018
@nathandunn
Copy link
Contributor Author

screen shot 2018-01-24 at 4 34 45 pm

HTMLFeatures don't seem to picking up the scale properly put canvas scale works

@nathandunn nathandunn changed the title Fix tabix histogram Fix tabix histogram for CanvasFeatuers Feb 1, 2018
@nathandunn nathandunn added this to the 1.12.4 milestone Feb 1, 2018
@nathandunn
Copy link
Contributor Author

Fixes #935

@nathandunn nathandunn requested review from rbuels and cmdcolin February 6, 2018 03:39
@nathandunn
Copy link
Contributor Author

@cmdcolin / @rbuels I think this is ready for review.

@nathandunn
Copy link
Contributor Author

This also has a fix for 'addStores', which should have been done within the master branch, but it ended up here. It is the correct fix, however.

@nathandunn nathandunn modified the milestones: 1.13.0, 1.12.4 Feb 6, 2018
@@ -59,7 +59,7 @@ define(['dojo/_base/declare', 'JBrowse/Util/dot-object'], function (declare, dot
var storeTracks = {};
var storeBookmarks = {};
Object.keys(queryParams).forEach(function (queryParam) {
if (queryParam.indexOf('addStore\.') == 0) {
if (queryParam.indexOf('addStores\.') == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a block comment to the top of this file saying what this class does? For posterity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added doc at the top. Thanks.

}, errorCallback);
},

_count: function( ref, min, max, itemCallback, finishCallback, errorCallback ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this function doesn't look right. it is returning count synchronously, but it also calls an error callback.

and it's returning a count of the blocks in the range, not the lines. what's up here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea. I removed it as they weren't actually used.

@nathandunn
Copy link
Contributor Author

@rbuels Ready for re-review assuming that everything passes.

@nathandunn nathandunn merged commit 7cd40bb into GMOD:master Feb 6, 2018
@ghost ghost removed the in progress currently being worked on label Feb 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants