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

Make edge thickness consistent across layouts and visualizations #276

Open
ElDeveloper opened this issue Jul 24, 2020 · 2 comments
Open
Labels
barplots bug needs-js-layouts Needs #213 to be taken care of first

Comments

@ElDeveloper
Copy link
Member

When coloring the tree and changing the width of the lines the thickness will be dependent on the tree, and the layout. It would be nice if this wasn't the case.

See #144 and #135 for more discussion.

fedarko added a commit to fedarko/empress that referenced this issue Jul 28, 2020
eventually thickness should be determined by assessing the "between-
tip distance" (i.e. how much is y=1 expressed in terms of the scaled
rect layout?), and using that to impact barplot positioning. for
circular layout i imagine something similar with "how much is
theta = 1?" or something vaguely like that.

wait does that relate to biocore#276? probably.
@fedarko fedarko added bug needs-js-layouts Needs #213 to be taken care of first labels Aug 13, 2020
@fedarko
Copy link
Collaborator

fedarko commented Aug 13, 2020

As more motivation to improve this -- the current method for scaling line widths uses the log of the number of leaves in the tree as a denominator in the equation. A problem with this is that if there's only 1 leaf in the tree (as in the silly corner-case where the tree is a straight line from the root to the single tip), then log(1) = 0, and none of the line widths show up as a result.

image

Having the layouts computed in JS will make recording the scaling factors used a lot easier, which should be the way to fix this. Hopefully.

fedarko added a commit to fedarko/empress that referenced this issue Sep 21, 2020
Relevant to biocore#276. The reliance on max displacement (which doesn't
seem to have an analogue in the unrooted layout? or maybe we can sort
of treat it the same way as circ layout, idk) means that this might
be too specific a solution to apply to line width thicknesses, but
it should make using barplots a lot less painful
@fedarko
Copy link
Collaborator

fedarko commented Nov 3, 2020

Also applies to:

  • barplot lengths (I have a fork that addresses this, need to polish it and file a PR)
  • node circles (ideally would be scaled with zoom level, and/or scaled to some optimal size within the SVG export)

ElDeveloper pushed a commit that referenced this issue Nov 10, 2020
… barplot lengths look thicker; fix a small Colorer bug (#442)

* ENH: Scale barplot lengths by max displacement

Relevant to #276. The reliance on max displacement (which doesn't
seem to have an analogue in the unrooted layout? or maybe we can sort
of treat it the same way as circ layout, idk) means that this might
be too specific a solution to apply to line width thicknesses, but
it should make using barplots a lot less painful

* BUG: precomp sm barplot lyr len&report correctly

* Document layout factor stuff

* add note re: 1.1

* BUG: Fix CLR_COL / "freebie" problem

* Remove unneeded comments re: freebie thing

* Test rgb2gl; prettify

* improve rgb2gl docs

* TST/MNT: Improve/test layout supported & freebies

* Do away with CLR_COL_GL, and document stuff

should address @ElDeveloper comment

* BUG: Also apply barplot unit to border lens

WHOOPS

* TST: fix barplot border test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
barplots bug needs-js-layouts Needs #213 to be taken care of first
Projects
None yet
Development

No branches or pull requests

2 participants