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

Blacken the doctest code in docstrings #3857

Merged
merged 3 commits into from
Mar 14, 2020
Merged

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Mar 11, 2020

With a bit of preparation this is the result of running black-doctest on the repository. The tool itself is experimental but I didn't find any issues after manually reviewing the changes. The exact call was:

python -m blackdoc .

I'm not too sure what line length we use for docstrings (I used the default of 88 for this run), but if we'd like a different number we can just rerun the tool.

python -m pytest --doctest-modules xarray/core still fails but from what I saw these failures are not due to changes in this PR.

  • Passes isort -rc . && black . && mypy . && flake8
  • Fully documented, including whats-new.rst for all changes and api.rst for new API

pd.Timestamp(2001, 2, 1)]],
dims=['time'])
>>> da.sel(time='2001-01-01')
>>> da = xr.DataArray(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had thought the standard was a blank line after each result; is that a general standard or was I holding that incorrectly?
(no need to do it this round though)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had the same impression, but the docs say otherwise:

Expected output cannot contain an all-whitespace line, since such a line is taken to signal the end of expected output. If expected output does contain a blank line, put <BLANKLINE> in your doctest example each place a blank line is expected.

I think that means that we only need the newline if we want to continue with normal text.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. IIUC we can still have blank lines, but it becomes a preference rather than enforced.
FWIW I do find they make it much easier to read the examples!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sounds good. Let's do that in a new PR, though.

@max-sixty
Copy link
Collaborator

Looks great! Congrats on such a good tool!

xarray/coding/cftimeindex.py Outdated Show resolved Hide resolved
@dcherian dcherian merged commit cafab46 into pydata:master Mar 14, 2020
@dcherian
Copy link
Contributor

Thanks @keewis

@keewis keewis deleted the blacken-doctests branch March 16, 2020 19:18
dcherian added a commit to dcherian/xarray that referenced this pull request Mar 22, 2020
* upstream/master:
  add spacing in the versions section of the issue report (pydata#3876)
  map_blocks: allow user function to add new unindexed dimension. (pydata#3817)
  Delete associated indexes when deleting coordinate variables. (pydata#3840)
  remove macos build while waiting for libwebp fix (pydata#3875)
  Fix html repr on non-str keys (pydata#3870)
  Allow ellipsis to be used in stack (pydata#3826)
  Improve where docstring (pydata#3836)
  Add DataArray.pad, Dataset.pad, Variable.pad (pydata#3596)
  Fix some warnings (pydata#3864)
  Feature/weighted (pydata#2922)
  Fix recombination in groupby when changing size along the grouped dimension (pydata#3807)
  Blacken the doctest code in docstrings (pydata#3857)
  Fix multi-index with categorical values. (pydata#3860)
  Fix interp bug when indexer shares coordinates with array (pydata#3758)
  Fix alignment with join="override" when some dims are unindexed (pydata#3839)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants