[issue26092] doctest should allow custom sys.displayhook

[Sergey B Kirpichev]

Change by Sergey B Kirpichev :


--
pull_requests: +24342
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/25651

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Noam Yorav-Raphael]

Noam Yorav-Raphael  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Sergey B Kirpichev]

Sergey B Kirpichev  added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Noam Yorav-Raphael]

Noam Yorav-Raphael  added the comment:

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 
wrote:

>
> Sergey B Kirpichev  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 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Sergey B Kirpichev]

Sergey B Kirpichev  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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Noam Yorav-Raphael]

Noam Yorav-Raphael  added the comment:

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.

--
nosy: +noamraph

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Tim Peters]

Tim Peters  added the comment:

Oops!  Should be issue 8048.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Tim Peters]

Tim Peters  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Tim Peters]

Change by Tim Peters :


--
nosy: +noam, tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Sergey B Kirpichev]

Sergey B Kirpichev added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[R. David Murray]

R. David Murray added the comment:

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

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[SilentGhost]

Changes by SilentGhost :


--
nosy: +georg.brandl
versions: +Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26092] doctest should allow custom sys.displayhook

[Sergey B Kirpichev]

New submission from Sergey B Kirpichev:

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.

--
components: Library (Lib)
messages: 258115
nosy: Sergey.Kirpichev
priority: normal
severity: normal
status: open
title: doctest should allow custom sys.displayhook
type: behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com