Sort dictionaries for doctests independent of ipythons pretty print

[antonio-rojas]

This is a prerequisite for #28197 - ipython does no longer honor the DICT_IS_ORDERED variable since 7.10 [1].

[1] ipython/ipython@dffa2c3

For doctesting we overwrite the default pretty print function for dictionaries to sort the keys and behave exactly as before.

CC: @jhpalmieri @dimpase @tscrim @antonio-rojas

Component: refactoring

Author: Jonathan Kliem, Antonio Rojas

Branch/Commit: 68b10e1

Reviewer: François Bissey

Issue created by migration from https://trac.sagemath.org/ticket/29042

[antonio-rojas]

Branch: u/arojas/remove_sorting_of_dicts

[antonio-rojas]

Author: Antonio Rojas

[antonio-rojas]

Description changed:

--- 
+++ 
@@ -1 +1,4 @@
+This is a prerequisite for #28197 - ipython does no longer honor the DICT_IS_ORDERED variable since 7.10 [1] so tests output needs to be updated. This will break the affected doctests (but not functionality) on python2
 
+
+[1] https://github.com/ipython/ipython/commit/dffa2c33dea3ecc09256a66a6ddbc876ce0629a9

[antonio-rojas]

New commits:

264d83a

Remove dict sorting and update doctests

[antonio-rojas]

Commit: 264d83a

[nbruin]

comment:3

This is going to be quite fragile, because now you're encoding a rather arbitrary dictionary order into the doctests, where no order matters (in py3 it's probably insertion order; a change in code somewhere that changes insertion order would break tests again!).

Why not introduce a little utility function print_dict_sorted that recovers the sorted dictionaries? It would amount to

sorted( D.items(), key=lambda t:t[0])

but with an appropriate routine you could print the result in a more dict-compatible way so that you can fix each doctest with a single-line edit.

That way you end up with a more future-proof fix, and it makes total sense to insert such a print routine at least into the doctest namespace (but then almost by necessity also in the interactive namespace)

[antonio-rojas]

comment:4

Sounds reasonable but that's beyond my ability, so someone else will have to do it.

[mwageringel]

comment:5

The idiom sorted(D.items()) is already used in several places in the doctests. Alternatively,

sage: from pprint import pprint
sage: pprint(D)

could be used to print a dict sorted by keys, which also seems to take care of nested dictionaries.

On the other hand, if one really wants to keep the sorted output of dicts in general, another solution would be to add a displayhook during doctests that imitates what is currently done by IPython. This would be more robust than using pprint everywhere, as it is likely that new doctests depending on insertion order would be introduced in the long run otherwise.

[nbruin]

comment:6

Replying to @mwageringel:

The idiom sorted(D.items()) is already used in several places in the doctests. Alternatively,

sage: from pprint import pprint
sage: pprint(D)

could be used to print a dict sorted by keys, which also seems to take care of nested dictionaries

Thanks for the suggestion! The routine I was thinking about already exists!. I think in "examples" doctests, the pprint is a little more desirable, because its output looks like a dictionary: reading the example with the printed output barely has any additional cognitive load. For that purpose, it might be worth considering to inject pprint in the global namespace by default. I think it serves a real purpose now that IPython dict printing is no longer sorted.

On the other hand, if one really wants to keep the sorted output of dicts in general, another solution would be to add a displayhook during doctests that imitates what is currently done by IPython. This would be more robust than using pprint everywhere, as it is likely that new doctests depending on insertion order would be introduced in the long run otherwise.

True, but that would cause a deviation between doctest output and the output that the actual example in the command line would give. The IPython reason for dispensing with sorting dictionaries is a good one, so I would not be in favour of bringing back that hook in general.

I also think that there are legitimate reasons to make doctests that do print the dict in the "Py3" order, for instance if used to compare with the iteration order. In those cases it would be possible to formulate the doctests differently, no doubt, but I would rather not preempt such use. I think putting up with having to use pprint is reasonable, with the price that it's a magnet for fragile doctests (but in reality, doctests for lots of more complicated routines are hard to make non-fragile. By comparison I think the dict issue is relatively mild.

[mwageringel]

comment:7

Replying to @nbruin:

For that purpose, it might be worth considering to inject pprint in the global namespace by default. I think it serves a real purpose now that IPython dict printing is no longer sorted.

In that case, rather than injecting pprint, it might be better to improve the pretty print function that already exists in the global namespace, for example by adding a new keyword pretty_print(D, sort=True). IMO, this makes it clearer why the pretty printing is used. This is synonymous to show(D, sort=True).

True, but that would cause a deviation between doctest output and the output that the actual example in the command line would give. The IPython reason for dispensing with sorting dictionaries is a good one, so I would not be in favour of bringing back that hook in general.

This deviation already exists, as the IPython sorting of dicts is only active during doctests. That said though, I do not have a strong preference for either solution – though I agree it is generally desirable to keep doctest output as close as possible to the actual output.

[nbruin]

comment:8

Replying to @mwageringel:

In that case, rather than injecting pprint, it might be better to improve the pretty print function that already exists in the global namespace, for example by adding a new keyword pretty_print(D, sort=True). IMO, this makes it clearer why the pretty printing is used. This is synonymous to show(D, sort=True).

It seems pretty_print aims at a different target: its default is latex output in the command line (that's a bad default), and in general "this function chooses the highest-quality output supported by the user interface", so that doesn't quite hit the goals of doctests (which should be concise and human-readable).

If we do

dm = get_display_manager()
dm.preferences.text='plain'

we end up with better results for our doctesting purposes. But that would be worse than from pprint import pprint.