Don't immediately draw barplots when clicking the "Draw barplots?" checkbox, to avoid crashing #343
Assignees
Labels
Comments
fedarko
added a commit
to fedarko/empress
that referenced
this issue
Aug 20, 2020
This was referenced Aug 21, 2020
kwcantrell
pushed a commit
that referenced
this issue
Aug 31, 2020
…gth-scaling (#357) * ENH: draw circular layout fm barplots! (hacky tho) * BUG/ENH: correctly set "maxX" in c.bps+use 2 rects * ENH: make s.m. barplots work in circular layout how is this actually not that bad what * DOC: upd8 comment * DOC: update README re #297 * DOC: document polar -> Cartesian stuff * DOC/PERF: mark some places to speed up cbarplots since this kinda uh crashes the EMP tree LMAO * PERF: reduce unnecessary work in c.barplot drawing * PERF: chop out more work in c.sm.b. drawing * MNT: abstract some more c. barplot drawing code * DOC: document new util funcs for circ barplots * DOC: clean up inner/outer radius terminology * Typo fixes * MNT: max X -> max displacement in bp drawing code a more elegant way of representing this for circ barplots * DOC: document precomputed y/angle stuff * STY: pret * MNT: shorten nodeAngleInfo var to just angleInfo * MNT: Catch bad-layout case in _getNodeAngleInfo() * ENH: Update "barplot unavail" msg re circ barplots I would still like to add some tests to the coordinate functions, but I think this is sufficient to close #297. * PERF: don't draw barplots by dflt (close #343) * DOC: punctuation * TST: Test _getNodeAngleInfo() * TST: add early addCircularBarCoords tst * TST: finish x-coord circ bar coord test just gotta test y-coords, with a similar switch stmt * STY: prettify test * TST: finish _addCircularBarCoords() test * TST/MNT: abstract redundant test code also add approxEqual func which I FEEL LIKE I ALREADY ADDED but w/e * TST/DOC: ref toFixedIfy et al in js layout tsts * MNT: rm check for unsupported layouts in bpdrawing * DOC: update comments in drawBarplots() start * TST: Add more context/detail to circ bar comp tst * DOC: clarify comment * PERF: don't compute halfAngleRange w rect barplots Makes things a bit more efficient. * STY: prettify test error msg * MNT: abstract rect coord-adding to sep func * TST: test _addRectangularBarCoords() * ENH: add length barplot legends; close #354 (still gotta test and document the new code, ofc) * MNT: Simplify+doc layer drawing code re lenlegends * TST: Clarify test util funcs * DOC/TST: refurbish testing utilities docs * TST: unbreak assignBarplotLengths() tests * DOC/TST: polish up and test assignBarplotLengths esp the new functionality. but also i apparently wasn't testing non-numeric and numeric combos! that's wack. so now that's tested. * TST: unbreak circ layout js tests (since nodes are now just referred to by postorder positions) * DOC: document more length legend stuff * TST: test Legend.addLengthKey() * STY: fix improperly named variable - jshint thing * DOC: fix outdated comment thanks @ElDeveloper! * MNT: Simplify maxD computation in drawBarplots() Thanks @ElDeveloper! * DOC: fix old node desc in _getNodeAngleInfo() thanks @kwcantrell! * DOC: max displacement (not just x) in fm/sm coords Forgot to update the docs. Thanks @kwcantrell for pointing this out. Also changed a small block of code to just compute thisLayerMaxD once and not like 3 times (?? why was I doing that lol) * MNT: Compute max displacement in getLayoutInfo() So that this is only done once per layout. Addresses comment from @kwcantrell. * STY: add missing semicolon * TST: restructure approxDeepEqual funcs to use math ... and not stringification :P Default epsilon is 1e-5, but for a few tests I only bothered writing things out to 4 decimal places, so for these I use 1e-4. * TST/STY: clean up utilities for testing * DOC: tidy up max displacement wording * BUG: Fix error when searching for invalid name * DOC: update getNodesWithName() docs * TST: fix approxDeepEqualMulti docs thanks @ElDeveloper * DOC: Update maxDisplacement description thanks @kwcantrell * MNT: add .gitattributes file marking vendored code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This behavior isn't a problem when there's feature metadata provided (and the default barplot layer is just an empty red bar or something), but when there isn't feature metadata -- and barplots default to stacked barplots of sample metadata -- then the current behavior means that a stacked barplot of whatever the "first" sample metadata field is will be drawn. If the sample metadata field in question is not categorical and/or it has a ton of groups, then the resulting stacked barplot will take a super long time to draw and/or just straight-up crash the browser.
This is currently a problem with the EMP tree (~200k tips with the chunk of it I'm testing with): the first sample metadata field is
adiv_chao1, and trying to draw a stacked barplot of that just straight up crashes the page in Chromium. (I get aSIGTRAPerror, bizarrely enough. Didn't know what that meant.).By deferring drawing barplots until the user presses "Update", the user is responsible for choosing what field to use. (I guess this sort of matches the behavior Emperor plots with hundreds of thousands of samples -- IIRC trying to color by a similarly silly field there will also hang, but when the hang happens it's clear to the user why things hanged.)
Changing this in the code should really just boil down to removing this line of code, but we'll have to update the README as well since it mentions how a red bar pops up immediately (rather than on pressing Update).
The text was updated successfully, but these errors were encountered: