DOC: fix remaining doc files for refguide_check by pdebuyl · Pull Request #15720 · numpy/numpy

pdebuyl · 2020-03-07T16:31:14Z

Related to #14970

Fix remaining files for refguide_check.

doc/source/f2py/getting-started.rst
doc/source/reference/arrays.nditer.rst
doc/source/user/c-info.python-as-glue.rst

Notes:

There remained some formatting issues that I put in separate commits.
Most problems stemmed from cython or f2py modules that cannot be compiled on the fly with the current build/test system. Those are skipped, see DOC, TST: check docstrings in doc/*rst files #14970 (comment)
There was a separate discrepancy in arrays.nditer.rst where some examples made incorrect use of the "external_loop" flag.
In arrays.nditer.rst, the loop output was unintuitive in some places and was fixed by assigning the loop output to a dummy variable, as discussed here: DOC, TST Update doctests ref guide #15519 (comment)

With this PR, python tools/refguide_check.py --rst outputs

OK: all checks passed!

and the sphinx build proceeds without warning (apart from a minor data clipping warning for imshow).

After merging, the NumPy project should consider testing via pytest and suitable plugins, including astropy's pytest-doctestplus, see #15703 (comment) and migrating the doc accordingly.

I test pytest on the files I edited and most errors stemmed from whitespace issues or specific usages (matplotlib functions return a repr for the figure object, those are not included in the current docs).

The external_loop prevent the iterations to go over items, which is not the intended effect in the document.

This is likely a temporary measure to enable continuous integration testing of the docs.

Fix whitespace issues. Fix formatting for current NumPy version.

mattip · 2020-03-15T17:44:52Z

Rather than skipping the f2py tests we could theoretically try to execute them:

.. for doctests

  >>> import numpy.f2py, os
  >>> with open('fib1.f') as fid:
  ...     source = fid.read()
  >>> numpy.f2py.compile(source, modulename='fib1')

but this does not work. The doctests are executed in a temporary directory, and have no access to the fib1.f file.

pdebuyl · 2020-03-15T20:30:37Z

I noticed as well.

Running pytest --doctest-glob '*.rst' executes the tests with the path when executing the command as the current working directory for the tests. But this requires replacing refguide_check.py with pytest.

mattip · 2020-03-15T21:12:26Z

But this requires replacing refguide_check.py with pytest.

I am OK with skipping all the f2py compilation doctests for now then.

mattip · 2020-03-15T21:13:04Z

doc/source/reference/arrays.nditer.rst

    ...     print(x, end=' ')
    ...
-    0 1 2 3 4 5
+    0 1 2 3 4 5 


extra space at the end of the line

mattip · 2020-03-15T21:15:35Z

Other than some extra spaces at the end of lines, this looks good to me.

pdebuyl · 2020-03-15T21:38:43Z

Hi @mattip thank you for the feedback. There might be an issue with the end-of-line marker on github. I did put the extra spaces. Did you check with a local copy?

mattip · 2020-03-15T23:05:30Z

I did put the extra spaces

We don't really like extra spaces on the end of lines and refguide-check does not require them, so it would be better to remove them.

pdebuyl · 2020-03-15T23:12:36Z

I used pytest to check the spacing of all tests, and the end-of-line space was required here. If we do move to pytest ultimately, those will be required.

Let me know if I should remove them despite this perspective.

mattip · 2020-03-25T18:19:12Z

Isn't there a way to skip all the f2py doctests using the SKIPLIST in refguide-check? Also - please remove the spaces at the end of the lines.

rgommers · 2020-03-25T18:28:45Z

I agree, a bit of context:

Isn't there a way to skip all the f2py doctests using the SKIPLIST in refguide-check?

This (skiplist) is how it also works in SciPy, and it's nicer because #doctest: SKIP is visual noise in examples (which is what doctests are primarily).

Also - please remove the spaces at the end of the lines.

Rationale: someone else's editor will strip these spaces again. We should just fix the printing behavior if needed (it doesn't really seem needed now).

This reverts commit 65f86ef.

This reverts commit be6cb63.

This reverts commit 02c4e5c.

pdebuyl · 2020-03-25T21:37:51Z

changes done, I tested locally and wait for the CI checks.

Some SKIP remain in arrays.nditer.rst as a small part of that file only relies on a compiled extension. Some examples there also use timeit which is not valid outside of IPython/jupyter

pdebuyl · 2020-03-25T22:17:20Z

@mattip I have addressed the review items.

mattip · 2020-03-26T06:03:16Z

tools/refguide_check.py

    'doc/source/release',
    'c-info.ufunc-tutorial.rst',
+    'c-info.python-as-glue.rst',
+    'getting-started.rst',


I got scared for a second, until I realized that gettting-started.rst is not the general beginner guide, rather the one inside the f2py directory. Could you rename the file to f2py.getting-started.rst or something that would prevent a future conflict with a getting-started.rst file for another task?

Sure. Here, as a contributor I hesitated to rename a file.

mattip · 2020-03-26T06:11:09Z

doc/source/reference/arrays.nditer.rst

    100 loops, best of 3: 11.8 ms per loop

-    >>> np.all(sum_squares_cy(a, axis=-1) == np.sum(a*a, axis=-1))
+    >>> np.all(sum_squares_cy(a, axis=-1) == np.sum(a*a, axis=-1)) # doctest: +SKIP


All these skips are annoying. Would it work to

split the file in two, move all the cython code to arrays.nditer.cython.rst

add arrays.nditer.cython.rst to the skiplist

use an .. include:: arrays.nditer.cython.rst so that sphinx will render the documents as before

add .. for doctests comments just before the include and at the top of the new file to explain why this was done

Tentative update pushed

Place the Cython-dependent section in separate file. Ignore the new file from the doctests with an RST_SKIPLIST entry in refguide_check.py

mattip · 2020-03-26T17:51:58Z

Looks good now thanks. At some point we should refactor the redundant parts of arrays.nditer.rst and 'nditer' notes.

mattip · 2020-03-26T17:54:18Z

Thanks @pdebuyl

pdebuyl added 6 commits March 7, 2020 11:29

DOC: skip f2py test for refguide_check

02c4e5c

DOC: fix formatting of output and traceback

a21fddd

DOC: remove external_loop flags to nditer

836186a

The external_loop prevent the iterations to go over items, which is not the intended effect in the document.

DOC: skip cython-dependent test

1d00793

This is likely a temporary measure to enable continuous integration testing of the docs.

DOC: skip f2py-dependent test

65f86ef

This is likely a temporary measure to enable continuous integration testing of the docs.

DOC: fix formatting of doc tests

be6cb63

Fix whitespace issues. Fix formatting for current NumPy version.

charris added 00 - Bug 04 - Documentation component: documentation labels Mar 8, 2020

mattip reviewed Mar 15, 2020

View reviewed changes

doc/source/reference/arrays.nditer.rst Outdated

... print(x, end=' ')

...

0 1 2 3 4 5

0 1 2 3 4 5

Copy link

Member

mattip Mar 15, 2020

Choose a reason for hiding this comment

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

extra space at the end of the line

mattip added the triage review Issue/PR to be discussed at the next triage meeting label Mar 16, 2020

mattip added triaged Issue/PR that was discussed in a triage meeting and removed triage review Issue/PR to be discussed at the next triage meeting labels Mar 25, 2020

pdebuyl added 4 commits March 25, 2020 22:17

Revert "DOC: skip f2py-dependent test"

ca80b9c

This reverts commit 65f86ef.

Revert "DOC: fix formatting of doc tests"

1a8e005

This reverts commit be6cb63.

Revert "DOC: skip f2py test for refguide_check"

463a673

This reverts commit 02c4e5c.

ignore f2py dependent rst files

86752a6

mattip reviewed Mar 26, 2020

View reviewed changes

pdebuyl added 2 commits March 26, 2020 14:56

DOC: rename f2py file getting-started.rst

cc8d8e4

DOC: Split file arrays.nditer.rst

7a1094a

Place the Cython-dependent section in separate file. Ignore the new file from the doctests with an RST_SKIPLIST entry in refguide_check.py

mattip merged commit 1cb3871 into numpy:master Mar 26, 2020

pdebuyl mentioned this pull request Mar 27, 2020

DOC: migrate to pytest and astropy/pytest-doctestplus #15845

Open

Uh oh!

Conversation

pdebuyl commented Mar 7, 2020

Uh oh!

mattip commented Mar 15, 2020

Uh oh!

pdebuyl commented Mar 15, 2020

Uh oh!

mattip commented Mar 15, 2020

Uh oh!

mattip Mar 15, 2020

Choose a reason for hiding this comment

Uh oh!

mattip commented Mar 15, 2020

Uh oh!

pdebuyl commented Mar 15, 2020

Uh oh!

mattip commented Mar 15, 2020

Uh oh!

pdebuyl commented Mar 15, 2020

Uh oh!

mattip commented Mar 25, 2020

Uh oh!

rgommers commented Mar 25, 2020

Uh oh!

pdebuyl commented Mar 25, 2020

Uh oh!

pdebuyl commented Mar 25, 2020

Uh oh!

mattip Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

pdebuyl Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

mattip Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

pdebuyl Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

mattip commented Mar 26, 2020

Uh oh!

mattip commented Mar 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants