Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

view: Allow user to specify narratives directory #128

Closed
huddlej opened this issue Aug 31, 2021 · 3 comments
Closed

view: Allow user to specify narratives directory #128

huddlej opened this issue Aug 31, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@huddlej
Copy link
Contributor

huddlej commented Aug 31, 2021

Context
The nextstrain view command is our recommended solution for users to view their Auspice JSONs. Its current interface accepts a single directory that should contain Auspice JSON files to view. The corresponding auspice view command provides a --datasetDir argument that implements the same functionality.

However, auspice view also provides an argument, --narrativeDir, that allows users to specify a directory that contains narratives in Markdown format. The nextstrain view command does not provide a way for users to view their narratives.

Description
Provide a way for users to specify a narratives directory, to mirror the functionality provided by Auspice.

Possible solutions

  • add an optional --narrative-dir argument to nextstrain view that passes its value through to auspice view --narrativeDir
  • pass arguments after the first positional argument through to auspice the same way nextstrain build passes arguments through to Snakemake
@huddlej huddlej added the enhancement New feature or request label Aug 31, 2021
@tsibley
Copy link
Member

tsibley commented Sep 2, 2021

Ah, good catch! I think the best way to solve this ends up being pretty simple: add --narrativeDir . to the auspice command run by nextstrain view. This will allow users to colocate dataset JSONs and narrative Markdown files in the same directory they pass to nextstrain view.

  • add an optional --narrative-dir argument to nextstrain view that passes its value through to auspice view --narrativeDir

I prefer to keep options to a minimum, and since there's an alternate solution which doesn't require another user option, I'd prefer that.

  • pass arguments after the first positional argument through to auspice the same way nextstrain build passes arguments through to Snakemake

I prefer even more to avoid this sort of pass thru of arguments when possible, as it can be hard to explain to users who aren't familiar with the concept and it can make it harder to change the interface in the future because pass thru breaks the encapsulation otherwise provided.

tsibley added a commit that referenced this issue Sep 2, 2021
Previously the launched Auspice would either show baked in test
narratives or no narratives at all, depending on the Auspice version.

Resolves #128.
@tsibley
Copy link
Member

tsibley commented Sep 2, 2021

I've implemented this in #129.

tsibley added a commit that referenced this issue Sep 2, 2021
Previously the launched Auspice would either show baked in test
narratives or no narratives at all, depending on the Auspice version.

Resolves #128.
@tsibley
Copy link
Member

tsibley commented Nov 24, 2021

Problems with the solution in #129 led to a different solution which I think is better:

nextstrain view continues to take a single directory as its main argument.

  • If the directory given to nextstrain view contains an auspice/ directory, then use that as the dataset dir.
  • Otherwise, fallback to existing behaviour and expect the given directory to directly contain *.json dataset files.
  • If the given directory contains a narratives/ directory, then use that as the narratives dir.
  • Otherwise, fallback to the behaviour in this PR and expect the given directory to directly contain *.md files.

This is backwards incompatible, but only if you have some datasets in a directory which also has an auspice/ directory in it. This seems unlikely enough, and the upside is that if you're following our build directory layout conventions (auspice/ and optionally narratives/), then nextstrain view <build> Just Works, which is nice, and also aligns the directory argument for nextstrain view with that of nextstrain build.

At the same time, it supports an arbitrary directory containing a pile of dataset and/or narrative files (except for the narrow case of also containing an auspice/ or narratives/ directory).

tsibley added a commit that referenced this issue Dec 6, 2022
Previously the launched Auspice would either show baked in test
narratives or no narratives at all, depending on the Auspice version.

Wee bit of an oversight. orz

Resolves #128.
@tsibley tsibley mentioned this issue Dec 6, 2022
3 tasks
@tsibley tsibley closed this as completed in b9153a8 Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants