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

bpo-36675: Doc: Reveal doctest directives #23620

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

JulienPalard
Copy link
Member

@JulienPalard JulienPalard commented Dec 2, 2020

Doctest directive were hidden in HTML and PDF output, this PR show them back.

https://bugs.python.org/issue36675

@JulienPalard JulienPalard removed needs backport to 3.8 only security fixes needs backport to 3.9 only security fixes labels Dec 2, 2020
@JulienPalard
Copy link
Member Author

Backport won't be possible, we have older sphinx on older branches.

@JulienPalard
Copy link
Member Author

@andresdelfino I see you a lot on the doc those time, though you would be interested in proofreading this :)

@andresdelfino
Copy link
Contributor

@JulienPalard Sure!

@JulienPalard JulienPalard merged commit c8a10d2 into python:master Dec 15, 2020
@andresdelfino
Copy link
Contributor

@JulienPalard could you check the comment I made? I believe something is missing.

@JulienPalard
Copy link
Member Author

@andresdelfino Gladly, but, which comment?

Another bad idea is to print things that embed an object address, like ::
Another bad idea is to print things that embed an object address, like

.. doctest::

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps

Suggested change
:no-trim-doctest-flags:

is missing here?

Copy link
Contributor

Choose a reason for hiding this comment

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

The rendered docs doesn't show the doctest directive in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's better to keep it hidden in this case. The case is:

>>> id(1.0) # certain to fail some of the time  
7948648
>>> class C: pass
>>> C()   # the default repr() for instances embeds an address  
<C object at 0x00AC18F0>

the comment in the case is true: it's expected to fail. If we reveal the hidden doctest: +SKIP it's no longer expected to fail, the comment becomes erroneous.

The point of this example is to demo usefull cases for +ELLIPSIS, not +SKIP.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, ok :)

@andresdelfino
Copy link
Contributor

@JulienPalard sorry, I thought it was submitted, but I was seeing a draft.

@asottile
Copy link
Contributor

asottile commented Jan 5, 2021

This breaks the minimum version of sphinx listed in Docs/conf.py:

needs_sphinx = '1.8'

:no-trim-doctest-flags: was added in sphinx 3.2 whereas the minimum version in Docs/conf.py is 1.8: sphinx-doc/sphinx@784e763

@JulienPalard
Copy link
Member Author

@asottile that's right, let's bump this, thanks for noticing!

@asottile
Copy link
Contributor

asottile commented Jan 6, 2021

@JulienPalard there's been additional efforts in this release to maintain compatibility with 1.x so there might be competing opinions here

@JulienPalard
Copy link
Member Author

JulienPalard commented Jan 6, 2021

Ok so let's discuss this futher in an issue. I'm opening it → https://bugs.python.org/issue42843

@asottile
Copy link
Contributor

asottile commented Jan 6, 2021

sounds good -- I only noticed this because it broke the packaging for deadsnakes (on 20.04, 18.04 and 16.04) so I reverted this patch there: deadsnakes/python3.10@f69292e

@JulienPalard
Copy link
Member Author

I only noticed this because it broke the packaging for deadsnakes

Sorry for this :(

@asottile
Copy link
Contributor

asottile commented Jan 6, 2021

I only noticed this because it broke the packaging for deadsnakes

Sorry for this :(

no big deal, I'm considering dropping the documentation build since it's ~95% of my maintenance and I'm not sure anybody actually uses it.

JulienPalard added a commit to JulienPalard/cpython that referenced this pull request Jan 21, 2021
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@JulienPalard JulienPalard deleted the doctest branch October 9, 2021 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants