search

nextstrain / narratives

Narrative markdown files accessed via nextstrain.org/narratives/x
2 stars 10 forks source link

Fix narratives which inadvertently specify multiple datasets #8

Closed jameshadfield closed 4 years ago

jameshadfield commented 4 years ago

The original design of nextstrain narratives was that each "section" (paragraph) should define its own dataset, such that a single narrative could display different datasets (or the same one if each section specified the same dataset). This was never implemented -- i.e. in auspice currently only the dataset specified in the YAML frontmatter is ever used -- which has resulted in the inadvertent creation of narratives which specify different (and perhaps invalid) datasets.

In auspice PR https://github.com/nextstrain/auspice/pull/1164 @eharkins and @salvatore-fxpig have managed to get this working 🎉 Before this is released we will endeavor to "correct" as many narratives as possible, of which there are many in this repo.

P.S. This list was generated from the following bash code, which requires auspice to be running in a separate terminal and pointed at the narratives housed in this repo.

for fn in *.md; do
  prefix=$( echo ${fn} | tr '_' '/' | sed -e 's/\.md//' )
  n=$( curl http://localhost:4000/charon/getNarrative?prefix=${prefix} 2>/dev/null | jq 'map(.dataset) | unique | length ' )
  if [ ! ${n} = "" ] && [ ! "$n" -eq "1" ]; then
    echo "- [ ] ${fn} had ${n} different datasets";
  fi
done
eharkins commented 4 years ago

Regarding narratives with multiple datasets (in auspice), this is our desired behavior as put by @jameshadfield :

If someone has inadvertently specified a different dataset for slide x then either:

  1. That dataset is a typo, and it doesn’t actually exist. I suspect this is the most common. In this case, we should attempt to fetch it, and when it fails gracefully fallback to using the current dataset. We should also display a warning (either in the console or via the banner notifications)
  2. The dataset is actually valid. In this case we do change to the “new” dataset.

case 1. here is mostly done as soon as we merge https://github.com/nextstrain/auspice/pull/1167 (unless we prefer a banner notification) but case 2 should be fixed for each existing narrative with multiple datasets, since many existing narratives specify multiple valid datasets, e.g.

- [ ] ncov_sit-rep_2020-04-10.md had different datasets: [
  "ncov/2020-03-05",
  "ncov/2020-03-27",
  "ncov/2020-04-10",
  "ncov/global/2020-04-09",
  "ncov/global/2020-04-10"
]
- [ ] ncov_sit-rep_2020-04-17.md had different datasets: [
  "ncov/north-america/2020-03-05",
  "ncov/north-america/2020-03-27",
  "ncov/north-america/2020-04-17"
]
- [ ] ncov_sit-rep_2020-04-24.md had different datasets: [
  "ncov/2020-03-27",
  "ncov/2020-04-24",
  "ncov/africa/2020-04-24"
]

Merging https://github.com/nextstrain/auspice/pull/1164 will change the behavior of those narratives, unless we make sure they all specify the same dataset on each slide.

jameshadfield commented 4 years ago

9 will close this

emmahodcroft commented 4 years ago

Thank you for that incredible work James!!