gosling-lang / gosling.js

Grammar of Scalable Linked Interactive Nucleotide Graphics
https://gosling.js.org
MIT License
160 stars 27 forks source link

Track height of circular tracks is unclear but required #278

Open flekschas opened 3 years ago

flekschas commented 3 years ago

I noticed that the a track needs a width and height. In the linear layout the track height is directly applied but in the circular layout it seems to be unused (while still being required) as the centerRadius ultimately defines the track height.

I wonder if the height could either be made optional in the circular layout or if it could be used.

E.g., you can test it with this exampleA%5DE'styleB('outlineWidthB2)E'sizeQ20)E'strokeQ'gray')E'strokeWidthQ0.5)E'widthB5CE'heightB20L-)L%5D%0A)M'%23-%20%20AL--B!%20C00DffE%2CAGMYgenomic'%20)E--'HbI9JdKaL%0A-M'%2C%20N8ODPcQB('valueBRieldB'SeTCU56Va5XchromY'typeB'Z7_cdjfq6~8e%01~qj_ZYXVUTSRQPONMLKJIHGEDCBA-_>).

sehilyi commented 3 years ago

I think the current use of width and height in circular layouts is not that straightforward to users and wonder if there would be better and more intuitive approach.

Currently, width is used to determine the width and height of a circular view (i.e., width == diameter of a circular view + small padding). If two views are serialized (i.e., arrangement: "serial") the cumulative width of two views is being used. This was mainly decided to make the change of the same visualization in different layouts less extreme (e.g., when changing layout: "linear" to layout: "circular", the width of the view does not change).

Screen Shot 2021-03-01 at 11 09 20 PM

height is only being used to determine the proportional size of rings (views) if views are parallelized (i.e., arrangement: "parallel"). If height of the first view is 200 and the second view is 100, then the proportion of their ring sizes is 2:1 and the actual size is determined by considering several other parameters (e.g., centerRadius, spacing, width).

Perhaps, instead of indirectly using the width and height in circular layouts, it might be better to add a few circular-specific parameters.

wangqianwen0418 commented 3 years ago

If we treat the width and height as the width and height of the track, when we roll up a track (i.e., circular layout), the width becomes the perimeter of the circle and the height is outerRadius - innerRadius. In this case, we will not need centerRadius anymore. I am not sure whether this makes more sense to you

sehilyi commented 3 years ago

@wangqianwen0418 Thank you for suggestion. If we stick to use width and height in circular layouts, I think your suggestion might be the most intuitive mapping to users since it preserves the semantic meaning of width and height between two layouts.

I think one disadvantage in this approach is that (1) it might be still challenging to calculate the circumference for users (which would require them to adjust the parameters several times by looking at the output visualizations) and (2) values of width and and height used in linear layouts would not work in circular layouts often times. For example, tracks without having relatively large width will not work in circular layouts (i.e., outerRadius - innerRadius will be larger than radius calculated by width).

Our main rationale was to provide low viscosity, i.e., changing layouts do not need to require users to change other props, like width and height, if possible. And I think the current approach works well in this regard. One limitation, however, is that how we use width and height values may not be clear to users as discussed in this issue.

To allow users to fine-tune the visual parameters in circular layouts more easily, I think we can add additional circular-specific parameters, and if such parameters are not specified, width and height will be used as they are in the current form. For example, we can add one prop that determines outerRadius - innerRadius (I'm not sure what would be a right term for this).

What do you think about the overall idea?

wangqianwen0418 commented 3 years ago

Yes, I agree with you! How about using width as the track width (as demonstrated in your diagram) and height as outerRadius - innerRadius? Another option is to ignore height when layout is circular and just give each track a constant outerRadius - innerRadius. Both solutions make sense to me and can help us get rid of centerRadius (I think this variable can be confusing sometime).

sehilyi commented 3 years ago

Sounds good. I agree to allow users to specify the outerRadius - innerRadius – We can either have a separate parameter for that or use the height. Let's discuss this during the meeting.

wangqianwen0418 commented 3 years ago

How about using height by default, and use outerRadius and innerRadius when they are given

sehilyi commented 3 years ago

One concern I have regarding it is that height used in linear layouts might not work in circular layouts. For example, multiple tracks in a single view are vertically stacked oftentimes (e.g., "Corces et al. 2020" in our editor), and in such cases, the cumulative height of tracks will easily exceed the width. In other words, cumulative outerRadius-innerRadius becomes larger than the actual diameter.

This will require (1) users to change the height by themselves or (2) we can adjust the height internally. (The second approach is similar to what we currently do, but with centerRadius.)

flekschas commented 3 years ago

My two cents are the following:

  1. Let width be the horizontal extent of the track boundaries and not the length of the track. One thing that would be nice is if Gosling could lend itself to produce publication ready figures. As a paper authors I want to be able to precisely control how wide my figure, and therefore my tracks, are. So a view with one tracks of width 200 should always produce an image that's 200px wide.
  2. Make width and height optional for accessibility. We can pick sensible default values. Maybe I'm missing something but I don't see a reason why we have to enforce the user to set those values.
  3. How about treating height exactly the same in linear and circular layout unless the user modifies it behavior via a special property circularHeightRelative: true or circularHeightRatio: true something. Regarding @sehilyi's concern: Taking a linear stack of 1D tracks and circularizing it can lead to severe over-plotting without any adjustments but I think at least the logic behind the grammar will be obvious to the user. The length of the axis perpendicular to the sequence axis is defined by height. In the linear track this refers to the vertical axis. In the circular layout this refers the axis going outward from the center. If they want to change this behavior I think it's better to have the designer explicitly specify the desired behavior than guessing what they want to do because that can go wrong.
sehilyi commented 3 years ago

Thank you all for sharing your thoughts, @wangqianwen0418 and @flekschas. I think we have a consensus here, and we can update the grammar accordingly as follows:

  1. Use width as how it is currently used, i.e., width in circular layout == horizontal extent.
  2. Use height as the length of the axis perpendicular to the sequence axis in both circular and linear layouts. This will make the use of height more understandable to users.
  3. Allow users to use relative height. This will make code editing much convenient if one wants to switch layouts frequently. I think @flekschas' opinion on using an explicit flag property (e.g., relativeCircularHeight, default false) looks a better idea than having an additional property to specify the relative height of tracks. A side effect of using this approach would be that centerRadius will be used only when the flag is set to true because it is meaningless when using absolute height.
  4. Provide default values for width and height and make them optional.