VEuPathDB / plot.data

1 stars 0 forks source link

add option to remove NAs when calculating numBinsToBinWidth #172

Closed asizemore closed 2 years ago

asizemore commented 2 years ago

Resolves #169

@bobular was right! The NAs were causing issues with numBinsToBinWidth.

Currently we use this function on the xAxisVar in map markers only. We use it before creating the plot.data class, so we have to handle NAs when this function is run. Because it's an axis var and this function is run so early, it made sense to simply remove the NAs.

Still, most of our functions intentionally do not remove NAs by default, hence the default value of the new na.rm arg is FALSE.

I expect that we'll need to use numBinsToBinWidth more substantially when we implement discrete gradient colormaps in all vizs. Handling binning with overlay vars is more complicated because of the no data toggle. If we turn 'no data' on for strata vars and chose a discrete gradient colormap with a continuous overlay var that included NAs, would we want to simply remove NAs? Seems likely but worth answering independently... or just not allowing evilMode at all...

Actually maybe we should just talk about this in the data viz meeting. If we have a discrete colormap created from a continuous var with NAs, we can keep doing a no data toggle in scatter bc we're changing the marker, but not in box. In box we'd end up with, say, 8 boxes colored from the discrete colormap, and one no data box in gray. The gray no data box would not be visually distinct enough from the other 8 boxes to all individuals.

bobular commented 2 years ago

Yes to talking about discrete gradient colormaps + no-data handling at dataviz!

asizemore commented 2 years ago

Upgraded qa rserve to v3.0.3. As long as all the containers are building, @bobular you should see the update soon!