Skip to content

Commit

Permalink
Use sections for each recommendation
Browse files Browse the repository at this point in the history
This is consistent with the existing documentation style guide and a
similar Snakemake style guide¹ written by @tsibley.

This also allows for the addition of a table of contents, which is done
here.

I looked into making the section links future-proof to wording changes,
however could not find a good solution. The closest is using :ref: roles
which adds separately defined links to to the section titles, but those
links aren't used by the TOC or header links themselves.

¹ https://github.com/blab/styleguide/blob/6aa5d7aa42acfa97e57a5ee05a4175f158502cac/nextstrain-builds.md
  • Loading branch information
victorlin committed Jul 25, 2023
1 parent 160e644 commit 57fe188
Showing 1 changed file with 33 additions and 26 deletions.
59 changes: 33 additions & 26 deletions src/reference/snakemake-style-guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,44 @@ Snakemake style guide
When in doubt, refer to `Snakemake's own best practices
guide <https://snakemake.readthedocs.io/en/stable/snakefiles/best_practices.html>`__.

- Avoid ``run`` blocks; implement custom Python in scripts called in a
``shell`` block.
.. contents:: Table of Contents
:local:

- Code in ``run`` blocks is less reusable. Anything we write once,
we're likely to want to reuse later.
Avoid ``run`` blocks
====================

- ``run`` blocks can be challenging to debug.
Instead, implement custom Python in scripts called in a ``shell`` block.

- ``run`` blocks do not run in rule-specific conda environments,
forcing the user to manually install into their environment any
dependencies that could have been in a conda environment file.
- Code in ``run`` blocks is less reusable. Anything we write once,
we're likely to want to reuse later.

- Define ``input`` paths with literal path strings instead of ``rule``
variables.
- ``run`` blocks can be challenging to debug.

- Literal paths are easier to read and interpret, avoiding the need
to trace back through a workflow to an earlier rule to see the
path associated with a rule output.
- ``run`` blocks do not run in rule-specific conda environments,
forcing the user to manually install into their environment any
dependencies that could have been in a conda environment file.

- Literal paths also allow workflows to be rewired with custom rules
that are injected at runtime. For example, `the ncov workflow
allows users to define their own
rules <https://docs.nextstrain.org/projects/ncov/en/latest/reference/configuration.html#custom-rules>`__
that can provide alternate commands for generating required files.
This approach does not work with references to rule outputs,
though (`see ncov PR
877 <https://github.com/nextstrain/ncov/pull/877>`__, for an
example).
Define ``input`` paths with literal path strings
================================================

- Avoid the ``message`` rule attribute.
Do this instead of using ``rule`` variables.

- When the ``message`` attribute is defined, Snakemake suppresses
other critical details that otherwise get displayed by default
(e.g., job id, rule name, input, output, etc.).
- Literal paths are easier to read and interpret, avoiding the need to
trace back through a workflow to an earlier rule to see the path
associated with a rule output.

- Literal paths also allow workflows to be rewired with custom rules
that are injected at runtime. For example, `the ncov workflow allows
users to define their own rules
<https://docs.nextstrain.org/projects/ncov/en/latest/reference/configuration.html#custom-rules>`__
that can provide alternate commands for generating required files.
This approach does not work with references to rule outputs, though
(`see ncov PR 877 <https://github.com/nextstrain/ncov/pull/877>`__,
for an example).

Avoid the ``message`` rule attribute
====================================

- When the ``message`` attribute is defined, Snakemake suppresses other
critical details that otherwise get displayed by default (e.g., job
id, rule name, input, output, etc.).

5 comments on commit 57fe188

@tsibley
Copy link
Member

Choose a reason for hiding this comment

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

I looked into making the section links future-proof to wording changes, however could not find a good solution. The closest is using :ref: roles which adds separately defined links to to the section titles, but those links aren't used by the TOC or header links themselves.

Hmm, I thought there was a way to have this:

.. _foo:

Foo bar baz
==========

and make Sphinx use the explicit foo target for the heading instead of auto-generating a target (e.g. as foo-bar-baz).

@victorlin
Copy link
Member Author

Choose a reason for hiding this comment

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

@tsibley that's what I'm looking for, but I can't figure out how to replace the auto-generated targets used by the TOC and header links. Let me know if you find a solution!

@tsibley
Copy link
Member

Choose a reason for hiding this comment

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

Ok, in experimenting and looking at Sphinx's configuration options, it's not doable with just those. I thought it was! But I guess I misremembered html_permalinks option or something. It'd be a pretty short docutils transformer, so I went looking for Sphinx extensions. Lo and behold, there's one that does exactly this: sphinx-better-subsection. That led me to the Sphinx issue about these ids.

@tsibley
Copy link
Member

Choose a reason for hiding this comment

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

I'll incorporate that into our Sphinx theme…

@tsibley
Copy link
Member

Choose a reason for hiding this comment

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

Please sign in to comment.