search

owid / owid-grapher

A platform for creating interactive data visualizations
https://ourworldindata.org
MIT License
1.38k stars 229 forks source link

Assigning manual entity colors doesn't work on some distinct bar charts #1671

Closed danyx23 closed 6 months ago

danyx23 commented 2 years ago

Description

When you have a distinct bar chart and you set an entity color in the data tab the corresponding bar should have this color but instead it doesn't change color FOR SOME BARS (!)

Expected behaviour

A bar with a manually assigned entity color should be drawn in that color

Steps to reproduce

Steps to reproduce the behavior:

  1. Open chart 1882 (e.g. here: https://staging.owid.cloud/admin/charts/1882/edit )
  2. In the data tab change the color for Switzerland
  3. The bar does not change color
  4. Change the color for South Korea
  5. The bar does change color

Screenshots

Screenshot 2022-09-20 at 15-08-10 owid-admin

Additional context

I was suspecting that I introduced this bug as part of the #1611 issue but the problem seems to predate this. Maybe it is related to the PR #1623?

marcelgerber commented 2 years ago

This is not related to any recent change - it's also happening on playfair (51712f2) and neurath (1c7398f), which are both running on many-months-old changesets right now.

From a very first investigation, it looks like entityIds are being dropped somewhere in the table transform, probably in interpolateColumnWithTolerance.

image

marcelgerber commented 2 years ago

Okay, so the good thing is that this is definitely not a new thing, but has probably been happening ever since Project Next. It's only happening for DiscreteBarCharts with tolerance, and only on the tolerance-induced entities.

The underlying problem is that CoreTable.complete([entityNameColumn, timeColumn]) doesn't include "related" columns like EntityId, EntityCode, EntityColor etc. for "newly-created" rows. We have tracked this issue before, and there is an old closed PR for it.

danyx23 commented 2 years ago

You make Sherlock Holmes look like Regular Holmes! Great that you uncovered where this came from, that is already very helpful.

larsyencken commented 2 years ago

Marked it as "nice to have" since it hasn't emerged as a prio for authors, but @marcelgerber if you believe it would be a quick fix you're welcome to have a shot after your other tasks.

marcelgerber commented 2 years ago

but @marcelgerber if you believe it would be a quick fix you're welcome to have a shot after your other tasks.

Definitely not an easy one, sadly!

larsyencken commented 1 year ago

Bumping it back down to "nice to have" since it hasn't caused us any issues in the meantime and no authors are banging down our door

github-actions[bot] commented 6 months ago

This issue has had no activity within 10 months. It is considered stale and will be closed in 7 days unless it is worked on or tagged as pinned.