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

docs: Improve TSV examples #1638

Merged
merged 4 commits into from
Mar 14, 2023
Merged

docs: Improve TSV examples #1638

merged 4 commits into from
Mar 14, 2023

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Mar 10, 2023

See commit messages.

Related issue(s)

#1588, #1588 (comment)

Testing

@nextstrain-bot nextstrain-bot temporarily deployed to auspice-trs-docs-tsv-kh92iokcw March 10, 2023 22:00 Inactive
tsibley added 4 commits March 10, 2023 14:00
These examples may have used CSV at one point, but now the whole section
is focused around TSV.
These were inadvertently lost in "Convert markdown pages to
reStructuredText" (7b8e45e) which converted them to spaces.

Note that these tabs still get expanded to spaces when rendering the rST
to HTML, and thus copying the example from the HTML page to paste into a
file will not produce a TSV.  I'll address this in a subsequent commit.
Using the literalinclude directive to include an external file preserves
literal tabs (unless the tab-width option is used to expand them).  This
resolves the copy and paste issue noted in "docs: Restore literal tabs
to the TSV examples in the rST source" (1275eb3).
This makes the TSV examples more readable on the page (i.e. the columns
line up unlike the raw TSV) while still retaining the ability for a
reader to get a copy of the example file to tinker with.

We could use the tab-width option of the literalinclude directive to
line up the columns, but that would break the ability to copy/paste the
contents into a new TSV file, as the tabs get expanded to spaces when
tab-width is used.
@tsibley tsibley temporarily deployed to auspice-trs-docs-tsv-kh92iokcw March 10, 2023 22:01 Inactive
Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slick! I learn something new about Sphinx every time you make a PR. Looks like I should be referencing Sphinx docs more, or just browsing it in general...

USVI/28/2016 B
USVI/41/2016 C
USVI/42/2016 C
strain secret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, thanks for catching this.

@tsibley
Copy link
Member Author

tsibley commented Mar 13, 2023

@victorlin

I learn something new about Sphinx every time you make a PR. Looks like I should be referencing Sphinx docs more, or just browsing it in general...

😊 \o/

Yeah, I think browsing official docs, in addition to using them for reference and search, is really underrated. The main reason is that browsing lets you build a mental index of what's available (and what's not), which is very useful before the reference/search phase of doc usage. (I'm not alone either.)

⁂ ⁂ ⁂

I'm going to wait a bit for @jameshadfield to chime in here since he had previous thoughts on these examples.

Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aesthetics are much improved here. I advised against tables as I wanted to reinforce that these are just plain-old text files, but the download button probably achieves this.

@tsibley
Copy link
Member Author

tsibley commented Mar 14, 2023

@jameshadfield Thanks for the 👀. Agreed good to reinforce these are plain text files! I think the download links maybe do that a bit less than inlined verbatim blocks, but also think that the improved reading experience of table + download is probably worth it.

@tsibley tsibley merged commit 8c278ae into master Mar 14, 2023
@tsibley tsibley deleted the trs/docs/tsv branch March 14, 2023 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants