chart-diff: there is some chart-leakage

lucasrodes commented 4 days ago

One-liner

After working on your PR for some time, if you check the chart-diffs, there might be some charts that are unknown to you. These seem to come from some inaccurate comparison between server database tables.

Things that lead to different charts

Different config. This works all the time
Different data. Mostly working
Different metadata. ⚠️ Has spurious differences

Context & details

After some days of working on your PR, some chart-diffs appear hidden in chart-diff app. These are ignored by @owidbot, and are not shown by default, so not critical. Most of the charts appear to be tagged with labels metadata change or data change.

Examples

PR https://github.com/owid/etl/pull/2861, with chart-diff available here.
PR https://github.com/owid/etl/pull/2867, chart-diff available here
PR https://github.com/owid/etl/pull/2872, chart-diff available here

Problems

Some datasets are different every time you build them (i.e. they are daily)
- This means they will always appear in the difference

Marigold commented 4 days ago

@lucasrodes I'm on it. I know where is this coming from. I'll rethink how modified_charts_by_admin works and split it into two parts - chart config updates and data/metadata updates.

larsyencken commented 1 day ago

From discussion

We should fix cases where the date in the metadata is dynamic, and take it from the snapshot instead
If that solves the problem, we stop there

owid / etl