DOC: Fix quickstart doc to pass refguide by pdebuyl · Pull Request #15703 · numpy/numpy

pdebuyl · 2020-03-04T13:08:48Z

Related to #14970

Fix refguide_check.py test for file doc/source/user/quickstart.rst

Fix printing of arrays.
Add doctest flag +IGNORE_EXCEPTION_DETAIL.
Fix view example with "c.base is a" by declaring "a" without reshaping.

I think that there is no controversial change in this one. The "view" example fails used to fail because a was defined by a reshape and did not "own" its data.

Fix printing of arrays. Add doctest flag +IGNORE_EXCEPTION_DETAIL. Fix view example with "c.base is a" by declaring "a" without reshaping.

doc/source/user/quickstart.rst

Co-Authored-By: Eric Wieser <[email protected]>

doc/source/user/quickstart.rst

Co-Authored-By: Eric Wieser <[email protected]>

pdebuyl · 2020-03-04T13:54:41Z

Hi @eric-wieser I actually go through the failures from refguide_check.py and submit the PR when it passes. Do you run a style checker for these whitespace issues?

eric-wieser · 2020-03-04T14:04:57Z

What you're seeing is that refguide only cares about the difference between no whitespace and some whitespace, rather than distinguishing different amounts of whitespace.

We changed the formatting of arrays back in 1.13, and a lots of the docs do not reflect that.

What I'd suggest you do in future is rerun the failing examples in a shell and pasting the exact output, rather than just tweaking the output to pass.

Co-Authored-By: Eric Wieser <[email protected]>

eric-wieser · 2020-03-04T16:59:42Z

doc/source/user/quickstart.rst

    Traceback (most recent call last):
-      File "<stdin>", line 1, in ?
+      ...
    IndexError: index (3) out of range (0<=index<=2) in dimension 0


This error message isn't accurate any more - can you update it to be correct?

eric-wieser · 2020-03-04T17:01:02Z

doc/source/user/quickstart.rst


    # not what we want
-    >>> a[s]
+    >>> a[s] # doctest: +IGNORE_EXCEPTION_DETAIL


Pretty sure this is not needed if you fix the error message below

This is right.

pdebuyl · 2020-03-04T17:01:07Z

copy-pasting every line of the file took quite long. I tested another method: I removed the NORMALIZE_WHITESPACE option in the pytest.ini file and run pytest on the rst file. This picks all whitespace errors so I could focus on those failures.

I'll prepare a PR after reviewing the changes manually though. pytest does not accept the "may vary" flags, etc, so there was a bit of a game to play.

eric-wieser · 2020-03-04T17:01:21Z

doc/source/user/quickstart.rst

-    >>> a += b                  # b is not automatically converted to integer type
+    >>> a += b                  # b is not automatically converted to integer type # doctest: +IGNORE_EXCEPTION_DETAIL
    Traceback (most recent call last):
        ...
-    numpy.core._exceptions.UFuncTypeError: Cannot cast from dtype('float64') to dtype('int64')  # IGNORE_EXCEPTION_DETAIL
+    numpy.core._exceptions.UFuncTypeError: Cannot cast from dtype('float64') to dtype('int64')


Is this necessary? What error message is refguide expecting?

The previous version returns

Traceback (most recent call last): File "/usr/lib/python3.7/doctest.py", line 1329, in __run compileflags, 1), test.globs) File "<doctest quickstart.rst[6]>", line 1, in <module> a += b # b is not automatically converted to integer type numpy.core._exceptions.UFuncTypeError: Cannot cast ufunc 'add' output from dtype('float64') to dtype('int64') with casting rule 'same_kind' /opt/default-py3-env/lib/python3.7/site-packages/numpy/core/_asarray.py:136: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray return array(a, dtype, copy=False, order=order, subok=True)

The doctest directive is not necessary

pdebuyl · 2020-03-04T22:06:44Z

I have fixed the whitespace issues.

I tried to be more strict about the textual output whereas I used to relax the testing earlier (i.e. by using the IGNORE_EXCEPTION_DETAIL flag or making use of the whitespace flexibility of the tests), with the intent of making the tests more robust to little changes.

eric-wieser · 2020-03-04T22:10:06Z

doc/source/user/quickstart.rst

-           [ 5,  6,  7,  8,  9],
-           [10, 11, 12, 13, 14]])
+	   [ 5,  6,  7,  8,  9],
+	   [10, 11, 12, 13, 14]])


This change looks wrong to me

I did a lot of copy-paste, tabs got mixed in by mistake. Funnily, it passes pytest and refguide_check. Fix coming.

eric-wieser · 2020-03-04T22:12:12Z

doc/source/user/quickstart.rst

           [[17, 16, 18, 20, 15],
            [14, 13, 15, 17, 12],
            [13, 12, 14, 16, 11]],
+    <BLANKLINE>


@mattip, did we delivery choose to avoid using BLANKLINE?

The recent file absolute_beginners.rst uses BLANKLINE, for instance. But refguide_check.py does not care, it is true. I was led into putting them by pytest.

We tried to work around <BLANKLINE> but there are places we couldn't avoid it. I don't remember the details. My conclusion when messing with it was that we should move to astropy's pytest-doctestplus rather than enhance refguide-check.

It seems you must either mash the blocks together without a blank line between them or use the <BLANKLINE> directive to indicate that formatting leaves a blankline there. The first is a bit misleading, I think I prefer the way it is now.

Should I attempt to resolve the remaining failures:

ERROR: failed checking doc/source/reference/arrays.nditer.rst failed checking doc/source/user/tutorial-svd.rst failed checking doc/source/user/c-info.python-as-glue.rst failed checking doc/source/f2py/getting-started.rst

Or should I test drive the astropy tool?

I think we should push forward with the current tool. Once we have cleaned up the last failures, we can add the check to CI, then the transition to the new tool can proceed in a controlled way.

eric-wieser

All looks good except the one comment, and a question for Matti

mattip · 2020-03-05T06:22:50Z

Thanks @pdebuyl

DOC: Fix quickstart doc to pass refguide

d9921cb

Fix printing of arrays. Add doctest flag +IGNORE_EXCEPTION_DETAIL. Fix view example with "c.base is a" by declaring "a" without reshaping.

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

pdebuyl and others added 4 commits March 4, 2020 14:50

Update doc/source/user/quickstart.rst

df5090d

Co-Authored-By: Eric Wieser <[email protected]>

Update doc/source/user/quickstart.rst

1d06206

Co-Authored-By: Eric Wieser <[email protected]>

Update doc/source/user/quickstart.rst

8eee69d

Co-Authored-By: Eric Wieser <[email protected]>

Update doc/source/user/quickstart.rst

c5d1600

Co-Authored-By: Eric Wieser <[email protected]>

eric-wieser reviewed Mar 4, 2020

View reviewed changes

doc/source/user/quickstart.rst Outdated Show resolved Hide resolved

Update doc/source/user/quickstart.rst

940da05

Co-Authored-By: Eric Wieser <[email protected]>

Apply suggestions from code review

51e6f3a

Co-Authored-By: Eric Wieser <[email protected]>

eric-wieser reviewed Mar 4, 2020

View reviewed changes

charris added 04 - Documentation component: documentation labels Mar 4, 2020

eric-wieser reviewed Mar 4, 2020

View reviewed changes

Fix whitespace and exceptions issue following review

e43ce4c

eric-wieser reviewed Mar 4, 2020

View reviewed changes

replace tabs with spaces

c620aed

mattip merged commit 459bfb4 into numpy:master Mar 5, 2020

eric-wieser mentioned this pull request Mar 5, 2020

DOC, TST: check docstrings in doc/*rst files #14970

Closed

14 tasks

pdebuyl mentioned this pull request Mar 7, 2020

DOC: fix remaining doc files for refguide_check #15720

Merged

pdebuyl mentioned this pull request Mar 27, 2020

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

Open

Uh oh!

Conversation

pdebuyl commented Mar 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pdebuyl commented Mar 4, 2020

Uh oh!

eric-wieser commented Mar 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdebuyl commented Mar 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdebuyl commented Mar 4, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattip Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser left a comment

Choose a reason for hiding this comment

Uh oh!

mattip commented Mar 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mattip Mar 5, 2020 •

edited

Loading