doctest should allow custom sys.displayhook

[skirpichev]

BPO	26092
Nosy	@tim-one, @birkenfeld, @bitdancer, @skirpichev, @noamraph
PRs	gh-70280: allow using custom sys.displayhook's with doctest #25651
Files	doctest-displayhook.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2016-01-12.18:20:57.316>
labels = ['type-bug', 'library']
title = 'doctest should allow custom sys.displayhook'
updated_at = <Date 2021-04-27.05:39:06.128>
user = 'https://github.com/skirpichev'

bugs.python.org fields:

activity = <Date 2021-04-27.05:39:06.128>
actor = 'Sergey.Kirpichev'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2016-01-12.18:20:57.316>
creator = 'Sergey.Kirpichev'
dependencies = []
files = ['49985']
hgrepos = []
issue_num = 26092
keywords = ['patch']
message_count = 10.0
messages = ['258115', '258548', '265468', '342680', '342681', '342688', '391882', '391896', '391898', '391900']
nosy_count = 6.0
nosy_names = ['tim.peters', 'georg.brandl', 'noam', 'r.david.murray', 'Sergey.Kirpichev', 'noamraph']
pr_nums = ['25651']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26092'
versions = ['Python 3.6']

Linked PRs

• gh-70280: allow using custom sys.displayhook's with doctest #111871

[skirpichev]

The purpose of doctest's - verify interactive examples. But if
your users will use custom displayhook all the time (consider, as
examples CAS like SymPy or https://github.com/skirpichev/omg/) - why
you must show them examples with the builtin's displayhook?

After https://bugs.python.org/issue8048, sys.displayhook can't be
customized for doctest. I think, that decision was wrong and we
should have a better solution.

PS: In fact, right now this issue can be workarrounded if you instead
override sys.__displayhook__ before doctest call. But I believe
this "solution" has own problems.

[bitdancer]

What problem does replacing __displayhook__ provoke? What solution do you propose instead of 8048, which fixed a bug?

[skirpichev]

What problem does replacing __displayhook__ provoke?

I don't know for sure.

But the documentation says "These objects contain the original values of displayhook and excepthook at the start of the program." So, other code in stdlib may assume something about __displayhook__ value. I.e. it writes the values it gets to sys.stdout. (Apparently, doctest thinks so.) Such assumptions should be documented, before people can override __displayhook__ safely.

What solution do you propose instead of 8048, which fixed a bug?

IMHO, it's not a bug. Why not override sys.displayhook just for doctests in this application?

PS:
Sorry for late answer, somehow I haven't got mail notification after your reply.

[tim-one]

Noam Yorav-Raphael, I tried to add you because this is pushing back against your patch in bpo-8408. It's been some years now, and nobody has cared enough to pursue it, so I'll just close this if you still don't care ;-)

As doctest's original author, I appreciate why 8408 was done, but don't think it was "a good" solution. In fact doctest can have no idea whether an example was _intended_ to be run with or without a custom shell's displayhook function invoked to massage the output first. So more sensible would have been to add a new doctest directive + optional argument, to make the intent explicit.

Which is certainly more work, and hasn't actually come up as "a problem" yet.

[tim-one]

Oops! Should be bpo-8048.

[noamraph]

Tim, thanks for letting me know. I certainly don't mind if you close this bug, since it undoes my old (and still relevant) fix.

[skirpichev]

Tim, lets decide on this simple issue.

To me, https://bugs.python.org/issue8048 was obviously a bad thing. While it "fixes" one application, which customize sys.displayhook in a strange way - it break testing almost everyone, which do sys.displayhook customization. See e.g. https://github.com/sympy/sympy/blob/master/conftest.py or https://github.com/diofant/diofant/blob/master/conftest.py. BTW, SymPy is far more popular library than dreampie, which is py2-only and looks unmaintained.

Last, but not least - introduced doctest's behaviour wasn't documented. It break things in a surprising way and do this silently... There is a documentation issue if you decide to keep this "feature".

Noam, what do you think about fixing your problem with mock.patch?

    >>> import sys
    >>> from unittest.mock import patch
    >>> with patch('sys.displayhook', sys.__displayhook__):
    ...     doctest.testmod()

Tentative patch attached.

[noam]

Hi,

I think that using mock.patch to fix the problem is fine. I personally
haven't encountered this problem in the past years, so whatever you decide
is fine by me.

Thanks!
Noam

On Mon, Apr 26, 2021 at 10:08 AM Sergey B Kirpichev <report@bugs.python.org>
wrote:

Sergey B Kirpichev <skirpichev@gmail.com> added the comment:

Tim, lets decide on this simple issue.

To me, https://bugs.python.org/issue8048 was obviously a bad thing.
While it "fixes" one application, which customize sys.displayhook in a
strange way - it break testing almost everyone, which do sys.displayhook
customization. See e.g.
https://github.com/sympy/sympy/blob/master/conftest.py or
https://github.com/diofant/diofant/blob/master/conftest.py. BTW, SymPy
is far more popular library than dreampie, which is py2-only and looks
unmaintained.

Last, but not least - introduced doctest's behaviour wasn't documented.
It break things in a surprising way and do this silently... There is a
documentation issue if you decide to keep this "feature".

Noam, what do you think about fixing your problem with mock.patch?

\>\>\> import sys
\>\>\> from unittest.mock import patch
\>\>\> with patch('sys.displayhook', sys.\_\_displayhook__):
...     doctest.testmod()

Tentative patch attached.

----------
keywords: +patch
Added file: https://bugs.python.org/file49985/doctest-displayhook.diff

---

Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue26092\>

---

[skirpichev]

I personally haven't encountered this problem in the past years

Noam, that's because Python has your patch :-)

But if we revert one - mentioned solution with mock.patch will work. Please, tell us if you find it bad for any reason.

[noamraph]

Yes, sorry, I didn't remember the history exactly.

I don't have a strong opinion. I'm okay with reverting the behavior to use sys.displayhook.

Thanks,
Noam

[ezio-melotti]

To summarize:

• In 2010 @noamraph created doctest assumes sys.displayhook hasn't been touched #52296 after https://bugs.launchpad.net/dreampie/+bug/530969 was reported.
• The issue was fixed in 25fbb89 and the change ensured that doctest always uses sys.__displayhook__.
• In 2016 @skirpichev created this issue, suggesting to revert the commit.
• In 2019 @tim-one said in a comment above that "As doctest's original author, I appreciate why 8408 was done, but don't think it was "a good" solution. [...] So more sensible would have been to add a new doctest directive + optional argument, to make the intent explicit."
• In 2021 @noamraph said that he's ok with reverting the commit (previous message), and @skirpichev created a PR to revert it: gh-70280: allow using custom sys.displayhook's with doctest #25651

[serhiy-storchaka]

I think that the fix in 25fbb89 was correct. Doctest runs the code in slightly different environment than the pure Python. Doctest patches several other global attributes: sys.stdout, pdb.set_trace, linecache.getlines. It is necessary to make doctest working. I think that perhaps more global attributes need to be patched, because more global hooks and parameters affecting the execution was added in recent years. But we simply did not have requests for other global attributes.

@skirpichev, would explicit setting sys.displayhook in your doctests help you? E.g.:

    """
    >>> from diofant.interactive.printing import init_printing
    >>> init_printing(pretty_print=True, no_global=True)
    >>> sqrt(5)
      ___
    ╲╱ 5
    """

Note that simply reverting 25fbb89 will break not only original OP's code, but will likely break doctests which set sys.displayhook explicitly (because it will be leaked outside of the particular test) and may break doctests for packages which set sys.displayhook implicitly, but do not set it for particular doctests.

[skirpichev]

would explicit setting sys.displayhook in your doctests help you?

@serhiy-storchaka, probably yes. See the current workaround, same uses e.g. the SymPy. But putting this stuff to every doctest will be too verbose... That's why I think that the doctest module should honor the sys.displayhook value.

Doctest patches several other global attributes: sys.stdout, pdb.set_trace, linecache.getlines. It is necessary to make doctest working.

Probably, so. But any good reason why the doctest can't honor the sys.displayhook value?

but will likely break doctests which set sys.displayhook explicitly

I doubt. See this.

[serhiy-storchaka]

What are your thoughts about adding a new doctest directive or optional argument, as @tim-one suggested, to make the transition more smooth?

It should be noted that while unittest wrappers around doctest (DocTestCase, DocFileTest, DocFileSuite) support arguments setUp and tearDown for setting up the test environment and cleaning it up, the native doctest interface (testmod, testfile) does not support such feature. How are you going to set up sys.displayhook for tests in particular files without affecting doctests in other files and creating testing order dependency?

[skirpichev]

What are your thoughts about adding a new doctest directive or optional argument, as @tim-one suggested, to make the transition more smooth?

@serhiy-storchaka, unless I miss something, the suggestion was to implement the current workaround (for #52296) as an option (not a default). That's definitely a more work without clear benefits: we can restore old behaviour (as after #52296), using setUp() method.
BTW, do we agreed, that the current behaviour is not documented and the suggested in this issue change can't be considered as a backward compatibility break?

How are you going to set up sys.displayhook for tests in particular files

You are right, I use the pytest's doctest plugin, which uses lower-level doctest's interfaces. Perhaps, testmod/testfile should expose setUp/tearDown stuff. But I'm not sure that this should be handled by this issue.

[skirpichev]

Slightly different version: #111871 (sys.displayhook value will be restored at the end of doctest).