Don't draw node circles by default for large trees? #349
Labels
Comments
|
What about turning them off by default? |
|
Sounds good to me :) An added benefit of that is that, since node circles don't scale with zoom level, they can "overwhelm" the tree when it's zoomed out:
|
|
👍 those trees look great BTW! |
fedarko
added a commit
to fedarko/empress
that referenced
this issue
Sep 17, 2020
even for nodes without a specified name. Closes biocore#349 and closes biocore#348. TODO: improve selection menu interface for no-name nodes
kwcantrell
pushed a commit
that referenced
this issue
Sep 22, 2020
* MNT: SVG export code tweaks - Declare variables with "var" - exportSvg() -> exportSVG() - exportSVG_legend() -> exportSVGLegend() - other minor things * MNT/ENH: tidy export code; mv legends to viz right Progress on #303 * STY: rm redundant declaration * MNT/DOC: document legend svg; consistent var names * MNT: don't draw a black circle @ root anymore * MNT: remove apparently redundant dom arg in func * init vb text method skeleton * MNT/TST: Add "legendType" attr to Legend; test will be useful when representing arbitrary legends in svg export * MNT: Store main legend in Empress, not SidePanel * TST: add legend-main to test index.html should unbreak tests * STY: pret * MNT/TST: store title easily in legend obj & test again, will help svg export * MNT: Delegate legend SVG exporting to Legend Still gotta, like, test the bulk of this tho. also code needs formatting * MNT/STY: make legend export work; prettify Also changed exported legend appearance a bit (no rounded rects, bg is white, color squares have a black border, etc) * STY: pret * MNT: refactor svg code;attempt to fix viewbox junk legend still isn't properly accounted for in the viewbox; gotta fix. code is a lot prettier tho * STY: jshint thing * MNT: use consistent legend font in svg; fix width? the width is a sledgehammer solution; need to make it exactly correct. idk whats wrong * BUG: fix (most of the) viewbox pbms with svg Now legends are fit in to the viewbox, and things seem mostly ok. Turns out the problem was using tree width (and hgt, but the problems i was seeing were just from width b/c legend wasn't fitting in on the x-axis) alongside "max X" stuff. Just storing things in units of "max X", etc. and THEN doing the -> width conversion after computing the SVG of the tree and legends fixed things. Remaining problems: - Push down the legend a bit; it looks like there is like a pixel or two at the top of it (in the border) that isn't shown. - Remove extra blank space on the right edge of the legend. Likely due to (2 * 4) (aka 2 * NODE_RADIUS) stuff being overzealous in how I'm using it. - Remove extra blank space on bottom edge of the legend when it is taller than the tree svg (e.g. for moving pictures f.m. coloring on level 7 taxonomy). Likely for same reason as above. - The tree is still flipped upside down. wat. Also, I need to document the heck out of the new SVG functions (in both empress.js and legend.js), and add tests preferably. Then it's PR time? * BUG???: Negate y-coords in SVG export: fix #334 I have NO IDEA why this works. Maybe ... is Empress itself flipping the y-coordinates in the first place, and the SVG export was right all along??? argh. * STY: pret * ENH: UI mockup of PNG export/export inclusion opts Inclusion options discussed with @ElDeveloper this morning. For an initial PR i'll probably comment out a lot of this stuff (e.g. the barplots) but it should be adaptable as we continue to support more exporting stuff * ENH: more ui mockup stuff for exporting * MNT: remove unused read() func + skbio imports we still need to include skbio as a dep because _plot needs to reference skbio.OrdinationResult or something (and also some of the python tests load trees as skbio TreeNodes before converting them to bp), but none of the main Empress python code touches TreeNode now at least * ENH: by dflt don't draw circles; draw forall nodes even for nodes without a specified name. Closes #349 and closes #348. TODO: improve selection menu interface for no-name nodes * ENH: Polish UI for unnamed nodes - Add fancy description in selected node menu, rather than saying "Name: null" - Add note to BPTree.getNodesWithName() about dangers of having null as a key in BPTree._nameToNodes. Would be nice to test this case eventuallly ... * STY: prettify * DOC: clarify getNodeCoords docs * TST: unbreak getNodeCoords test for #348 * ENH: add early PNG export (close #330) Also comment out some unused stuff from the export panel for now * ENH: keep sel node shape as circle; node rad attrs See comments -- basically, when node circles were turned off, the selected node shape turned into a square. now it's always a circle which is nice ux-wise * ENH: p major refactor of svg stuff -Splitting up tree and legend exporting, per convo with @ElDeveloper -Make tree export (SVG and PNG) work nicely (SVG still needs barplot and collapsed clade support; that'll come soon!) -Add docs Just left is legend exporting, which shouldn't be too bad. knock on wood. That'll also include barplot legends, I think! * STY: prettify, and abstract rgb code to new func * ENH: polish up legend svg export + clean code fixed bug where all legends getting plomped on top of each other b/c maxy wasn't updated properly. still has slight bug where there's extra empty space below the bottom legend. not sure where originator is but it seems to be exactly 2*unit pixels * MNT: clean up leg exporting code a tiny bit * MNT: for now, don't export the same legend 3 times (was doing that to test multi legend junk but for now it's ok) * BUG: Fix extra v-space bug (kinda) in leg export * STY/DOC: prettify and upd8 comment re vspace thing * MNT: whoops only include one of the tree legend * DOC: document a lotta stuff * DOC: add note re negating * DOC: fix wording * DOC: document exporting fixes * DOC: word unnamed node message better * DOC: close a paren and add context to null thing * DOC: tidy up some comments * ENH: center legend titles in export * ENH: make legend colors snug, like in-app legends So, part of my confusion earlier was that rects (i.e. color squares) have their x and y describe their top left, but BY DEFAULT text tags in SVG have their x and y describe their bottom left. And you gotta use dominant-baseline to change that. Not sure what sadist approved that but ok * BUG/DOC: fix legend svg width;add docs;abstr code * STY: prettify~~~ * DOC: document funky legend stuff tbh i'm still kinda lost on why this works as well as it does but OH WELL :D * MNT: use SVG styles to cut down on redundant code * STY: prettify the styling i warned you about styling bro / i told you dog * MNT: Create context in Legend.exportSVG() removes some work from empress.js, and lets us adjust context based on legend font (shouldn't be dynamic but just in case :O) * DOC: font family -> font style more accurate description * MNT: Move exporting code to export-util.js * STY: preeettify * TST: unbreak JS tests (point to export-util.js) Also rm'd an extra log statement * DOC: Point Legend.exportSVG docs to ExportUtil due to code reorganization in PR * BUG: fix Emperor selection callback Noticed that double-clicking on a category didn't work with the biplot generated for this PR. turns out that that's b/c I forgot to update the way the legend is cleared in that file. should be good now * PERF: Clear node circle buffer with toggling Pair-programmed with @kwcantrell * BUG/MNT: rename drawer radii to diams & upd8 svg
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
At least on the EMP tree I was working with, toggling node circles took a noticeable amount of time (a few seconds IIRC). If this time is presumably being used on startup to draw node circles, I think we should save that time so that the initial loading period is as short as possible -- allowing users to draw node circles later, of course.
Not sure what a good threshold would be; maybe 100k tips? (We could also just disable node circles by default; IDK what would be ideal.)
The text was updated successfully, but these errors were encountered: