-
Notifications
You must be signed in to change notification settings - Fork 0
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
legend updates needed for divergent colormaps for continuous overlay variables #454
legend updates needed for divergent colormaps for continuous overlay variables #454
Comments
also, in the 1/25/2023 dataviz meeting, we decided that -- for diverging colormaps -- when the range isn't symmetrical (ie, |
Hi @danicahelb May I ask two questions to clarify? First, for the case of Bob's screenshot above, displayMinMax = (-30, 30) and rangeMinMax = (-15.3, 273.91), then should the new range be (-30, 273.91)? Second, do we need to consider subsetted (filtered) data? For example, assuming that the range of the displayMinMax and rangeMinMax are the same with the first question and somehow subsetted data has MinMax = (10, 20), then do we need to respect the combination of the displayMinMax and rangeMinMax (e.g., -30, 273.91) or just use data-based MinMax (10, 20) for the colormap range?
|
This seems complicated. I can't see a clear feature request in my original message. Can we clarify what we mean here:
So let's say a regular -25 to +25 color map goes from dark blue to dark red. Does the legend for -5 to +25 go from very light blue to dark red? |
DK's question about filtered ranges might depend on whether the overlay variable was directly or indirectly filtered - aka "filter-aware" behaviour? |
@bobular First of all, thank you for looking into this ticket 👍 Concerning your question, all legends (both normal and colormap) utilize pre-defined color scale from the first to the end. Thus, if the range will be -5 to +25, I think it should be from dark blue to dark red which would be more consistent throughout the style of colormap: that is, a minimum is dark blue and a maximum is dark red. |
@bobular Overlay variable contains metadata for rangeMin/Max and/or displayMin/Max. I have checked those parameters after subsetting/filtering, but they did not change. Actually this is the reason why we computed data-based min/max for Scatterplot, Lineplot, etc. (for axis range controls) Can you please confirm if you intended to consider filtered range as well? I think it should be like other Vizs (Scatterplot etc.) but I wanted to make sure of it before going for it because while it would be doable, it will take some time. I have already made a change to handle displayMin/Max and rangeMin/Max but data-based Min/Max in this case is somewhat tricky because we need to firstly compute data-based Min/Max (including facet case) and then change data based on the Min/Max (marker colors, etc.): on the other hand, for an example of Scatterplot Viz, we just used data-based Min/Max for setting axis range control, not changing data itself. |
Oh, I think it needs to use the middle (white-ish) color for zero. @danicahelb please could you confirm this and my question above. |
@moontrip you are correct the variable metadata doesn't change when we filter on that variable. I am jumping the gun on this issue. I think we need new tickets for filter-aware axis range and overlay colormap behaviour. I was just asking Danielle about it in #frontend-dev |
@bobular yes I am aware of the filter-aware metadata work by Danielle, as I left a comment in my PR, VEuPathDB/web-eda#1696 (comment): but I think it is still underway? That color mapping stuff is interesting. I didn't know we should stick to zero=white for a divergent colormap. I will see the decision and act 😃 |
Oh yes, sorry I didn't see (or did see, then forgot) that you were already on top of the filter-responsive gradient handling. The important point about filter-aware handling is that if the user has filtered directly on the variable that is used in the overlay/gradient then we use the filter's min/max (not the back end response-derived min/max).
There's no back end work to wait on here, so we could implement the filter-aware gradient handling right now, if you like. |
The filter-aware overlay gradient handling can also wait for a new ticket if you like. |
@bobular Thank you for your detailed information but I am afraid I am missing something or confused. If I understand correctly, you are mixing the term "filter-aware" in your explanation? There are two things concerning the term "filter-aware": a) filter-aware metadata/annotation (VEuPathDB/EdaDataService#240) - still draft PR; b) "filter-aware gradient handling" at the end of your sentence is the same with the "filter-responsive gradient handling" (computing data/response-based Min/Max) at the top sentence? |
Hi @moontrip Sorry for the delay getting back to you. I had meetings and then had to think about this carefully. Let's leave filter-awareness to the new tickets which I'll make soon. I have said some confusing things in this ticket, sorry about that. Let's rewind back to the initial specs that @danicahelb provided above: #454 (comment)
What this means is that we should not set the range based on the min/max of the response data, as is currently implemented in this PR. e.g. here I think what we need is to add a new option to Then we can use Then we can come back to the zero-straddling ranges, and what to do about the colors after @danicahelb has clarified the requirements I asked about here. |
@bobular thank you for your detailed information and suggestions. It is very helpful for me to figure out what we want to implement 👍 Below are my thoughts. First, data-based min/max is only used when filters is not empty: i.e., subsetted. It is because without filters, rangeMin/Max from metadata are essentially the same with data-based min/max. As I said in my PR, if there is no filter, I just compared displayMin/Max and rangeMin/Max. Second, from my understanding, lodash.min/max throws error like Math.min/max or return undefined if an undefined or null (variable/value) in the function arguments is used. e.g., Third, it is not clear to me if you want to send 'undefined' to useDefaultAxisRange() for observed Min/Max (data-based Min/Max) under current situation (without filter-aware metadata)? Certainly we can send observed Min/Max to the hook and forcibly set axisRangeSpec: 'Full' as an argument of the hook: just FYI, that is basically the same to what I have currently implemented in the PR for non-filter case. So basically, a question is how we handle filter/subset case for the range. |
Hi @moontrip @danicahelb didn't answer your question below, but I clarified yesterday with Dave/Danielle that the filter-aware behaviour is as described in this new ticket. Overlay variable color gradient ranges are never scaled to the min/max of the data response (i.e. similar to "auto zoom" mode for axis ranges)
The task for this ticket is to remove the "start at zero" for the overlay "axis". So lodash does return undefined for For your third question, I don't think it matters if we pass observed min/max or undefined into |
@bobular Thank you for your detailed explanations 👍 I didn't consider the case the array form with the lodash min/max. Will test it and change the hook and associated function: hope TS does not complain 😄 Will probably rework this PR early next week as I am currently engaged with other ticket(s). |
@bobular Out of curiosity, I just did a quick test for lodash min (or max) with the array form in the jsfiddle. But unfortunately, the array form still throws error if one of elements comprised of variables is not defined (i.e., non-existent). For example, a=1 & c=2, then _.min([a, b, c]) throws error because b is not defined. This will be the case if, e.g., displayMin does not exist in the data while data.value.rangeMin exists, then _.min([data.value.displayMin, data.value.rangeMin]) will still throw error. perhaps this can be avoided by checking the presence of displayMin/Max & rangeMin/Max by assigning them as new local variables, and then set them as undefined value if non-existent. |
Hi @moontrip - I tested in node interactive "shell".
But this is OK
I'm pretty sure we'll be passing a variable to |
@bobular Thank you for your test. Yes you are right. There are two different cases: a) JS variable is not defined; b) JS variable is defined, but its value is not defined (undefined) or null. So even |
From @bobular
In GEMS1, BMI-for-age z-score ranges from -15 to +273, but when i use this term as an overlay, the legend indicates it ranges from -30 to +30. (I thought it could have something to do with not having data for the participant whose BMI-for-age z-score was 273 but checked and this is not the case)
I've had a quick look. The code is creating a symmetrical gradient around zero and is using the curated
displayRangeMin
anddisplayRangeMax
in preference to the data-derivedrangeMin
andrangeMax
. (This preference has been standard practice until now, I believe.) So this is why we get -30 to 30 (because it creates a symmetric range usingmax(abs())
The text was updated successfully, but these errors were encountered: