classification
Title: doctest should allow custom sys.displayhook
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Sergey.Kirpichev, georg.brandl, noam, noamraph, r.david.murray, tim.peters
Priority: normal Keywords: patch

Created on 2016-01-12 18:20 by Sergey.Kirpichev, last changed 2021-04-27 05:39 by Sergey.Kirpichev.

Files
File name Uploaded Description Edit
doctest-displayhook.diff Sergey.Kirpichev, 2021-04-26 07:08
Pull Requests
URL Status Linked Edit
PR 25651 open Sergey.Kirpichev, 2021-04-27 05:39
Messages (10)
msg258115 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2016-01-12 18:20
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.
msg258548 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-01-18 21:10
What problem does replacing __displayhook__ provoke?  What solution do you propose instead of 8048, which fixed a bug?
msg265468 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2016-05-13 12:04
> 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.
msg342680 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2019-05-17 00:17
Noam Yorav-Raphael, I tried to add you because this is pushing back against your patch in issue 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.
msg342681 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2019-05-17 00:18
Oops!  Should be issue 8048.
msg342688 - (view) Author: Noam Yorav-Raphael (noamraph) Date: 2019-05-17 04:40
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.
msg391882 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2021-04-26 07:08
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.
msg391896 - (view) Author: Noam Yorav-Raphael (noam) Date: 2021-04-26 10:28
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>
> _______________________________________
>
msg391898 - (view) Author: Sergey B Kirpichev (Sergey.Kirpichev) * Date: 2021-04-26 10:48
> 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.
msg391900 - (view) Author: Noam Yorav-Raphael (noamraph) Date: 2021-04-26 10:59
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
History
Date User Action Args
2021-04-27 05:39:06Sergey.Kirpichevsetstage: patch review
pull_requests: + pull_request24342
2021-04-26 10:59:39noamraphsetmessages: + msg391900
2021-04-26 10:48:59Sergey.Kirpichevsetmessages: + msg391898
2021-04-26 10:28:35noamsetmessages: + msg391896
2021-04-26 07:08:37Sergey.Kirpichevsetfiles: + doctest-displayhook.diff
keywords: + patch
messages: + msg391882
2019-05-17 04:40:00noamraphsetnosy: + noamraph
messages: + msg342688
2019-05-17 00:18:59tim.peterssetmessages: + msg342681
2019-05-17 00:17:33tim.peterssetmessages: + msg342680
2019-05-17 00:16:33tim.peterssetnosy: + tim.peters, noam
2016-05-13 12:04:04Sergey.Kirpichevsetmessages: + msg265468
2016-01-18 21:10:04r.david.murraysetnosy: + r.david.murray
messages: + msg258548
2016-01-15 09:25:08SilentGhostsetnosy: + georg.brandl

versions: + Python 3.6
2016-01-12 18:20:57Sergey.Kirpichevcreate