owid-grapher icon indicating copy to clipboard operation
owid-grapher copied to clipboard

Cleanup: Remove ScatterPlot `hideLinesOutsideTolerance` option

Open marcelgerber opened this issue 2 years ago • 3 comments

Description

There is a special option for ScatterPlots called hideLinesOutsideTolerance. @danielgavrilov and I were unsure what it even does when we stumbled across it last week, which is a bad sign 🚫 Then we found out that only 8 charts currently use that feature.

In the admin, the setting can be accessed under Scatter > Hide entities without data for full time span (within tolerance).

What does it do?

That option was introduced in September 2017. If enabled, then we do roughly the following:

for country in countries:
	if (
	 	no data point for year minTime ± tolerance exists for country
	   or
	 	no data point for year maxTime ± tolerance exists for country
	   ):
			excludeFromView(country)

Can we remove it?

Given how few charts have this flag enabled, and how specific it is, I think we can remove it. But we should ask authors for their stance on it. The setting seems most important when Average annual change is enabled, because in that case it ensures that the average change is only calculated for countries which have lots of data, and where the calculated average is actually meaningful. We might want to enable a similar feature for the Average annual change mode, but without any config behind it.

The crassest example of that is this chart, where toggling the checkbox in the admin makes the chart shift around by a lot (that chart is never used).

In the screenshot, the setting is disabled (default) on the left and enabled (as in the config) on the right. image

Suggested steps

  • [x] Ask authors whether they need this feature
  • [ ] Decide what to do about Average annual change mode
  • [ ] Remove option
  • [ ] Write DB migration to remove that property from all existing configs

marcelgerber avatar May 17 '22 21:05 marcelgerber

@marcelgerber can you ask the authors about this? 😃

danyx23 avatar May 24 '22 11:05 danyx23

Just asked Hannah and Max about it (in a private chat). They will most likely be the only two people that know about the existence of this setting, so doesn't make sense to ask a larger group about it.

marcelgerber avatar May 24 '22 15:05 marcelgerber

Both Max and Hannah are okay with it being removed in the future. Hannah didn't even know the toggle existed 😁

Their responses

No, I didn't really know it existed, or at least I've never had it on my radar or used it. I don't think anyone in the team does (as per the fact that only 8 charts use it).

I'd say it's definitely okay to remove the setting. No one will even notice that it's gone.

-- Hannah

Yes, I agree with it. If it causes trouble or is hard to maintain then yes, let’s discontinue it.

-- Max

marcelgerber avatar May 24 '22 18:05 marcelgerber