Opened 2 years ago

Closed 2 years ago

#29042 closed enhancement (fixed)

Sort dictionaries for doctests independent of ipythons pretty print

Reported by: arojas Owned by:
Priority: critical Milestone: sage-9.2
Component: refactoring Keywords:
Cc: jhpalmieri, dimpase, tscrim, arojas Merged in:
Authors: Jonathan Kliem, Antonio Rojas Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 68b10e1 (Commits, GitHub, GitLab) Commit: 68b10e1e6665892edad775e6b9b18e93afc3d637
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

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

[1] https://github.com/ipython/ipython/commit/dffa2c33dea3ecc09256a66a6ddbc876ce0629a9

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

Change History (60)

comment:1 Changed 2 years ago by arojas

  • Branch set to u/arojas/remove_sorting_of_dicts

comment:2 Changed 2 years ago by arojas

  • Authors set to Antonio Rojas
  • Commit set to 264d83a88a5c661bfbf89366ec65b2f510c7eecb
  • Component changed from PLEASE CHANGE to refactoring
  • Description modified (diff)
  • Type changed from PLEASE CHANGE to enhancement

New commits:

264d83aRemove dict sorting and update doctests

comment:3 Changed 2 years ago by nbruin

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)

comment:4 Changed 2 years ago by arojas

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

comment:5 follow-up: Changed 2 years ago by gh-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.

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.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 2 years ago by nbruin

Replying to gh-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.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 2 years ago by gh-mwageringel

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.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 2 years ago by nbruin

Replying to gh-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.

comment:9 in reply to: ↑ 8 Changed 2 years ago by gh-mwageringel

Replying to nbruin:

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

I fail to see the rationale behind defaulting to latex, or the intended use case of pretty_print. Making a distinction between %display plain and %display None is bizarre and seems wrong to me. The usual REPL output at least does not make such a distinction (implemented in sage.repl.rich_output.display_manager.DisplayManager._preferred_text_formatter).

Should we add pprint to the global namespace then? I am not sure this is the best thing to do, but it seems practical. Or just add explicit from pprint import pprint statements in the doctests?

comment:10 Changed 2 years ago by mantepse

It seems to me that it would be good to discuss this at sage-devel, because it affects all developers.

Let me add that it is unclear to me what the best solution is. Initially, I liked the idea of pretty_print(D, sort=True). I think that yet another pretty print function is a bad idea, I find the existence of show, pretty_print, ascii_art, .pp() and possibly others confusing enough.

comment:11 Changed 2 years ago by git

  • Commit changed from 264d83a88a5c661bfbf89366ec65b2f510c7eecb to adcc47704f9f6725634c671746633a1a1aed663c

Branch pushed to git repo; I updated commit sha1. New commits:

f75b41fMerge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts
adcc477Fix two new tests in ring_extension.pyx

comment:12 Changed 2 years ago by git

  • Commit changed from adcc47704f9f6725634c671746633a1a1aed663c to 1f82c673d1420074b0b9073593d6ab4bc01d7049

Branch pushed to git repo; I updated commit sha1. New commits:

81c03c7Merge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts
1f82c67Sync with develop

comment:13 Changed 2 years ago by git

  • Commit changed from 1f82c673d1420074b0b9073593d6ab4bc01d7049 to 5c36c8761ea119c93dd7ae89a61eb4c16da9995a

Branch pushed to git repo; I updated commit sha1. New commits:

5c36c87Merge 9.1.beta7

comment:14 Changed 2 years ago by git

  • Commit changed from 5c36c8761ea119c93dd7ae89a61eb4c16da9995a to 71427f10863614bb7130822715ebb201ec087aa6

Branch pushed to git repo; I updated commit sha1. New commits:

71427f1Update for 9.1.beta7

comment:15 follow-up: Changed 2 years ago by fbissey

This needs rebasing. My own thinking is that calling the structure dictionary lead humans to have an expectation for it to be ordered the way the physical object is. In fact we have look up tables which are very handy for the computer to manipulate but not necessarily meant for full display. In most case sorting doesn't matter objectively.

So I am not sure we should compare whole dictionaries for doctests. Size of the dictionary and some key elements should be enough.

comment:16 Changed 2 years ago by git

  • Commit changed from 71427f10863614bb7130822715ebb201ec087aa6 to a86697aa41c0f45abb25fd589db90bae61e57268

Branch pushed to git repo; I updated commit sha1. New commits:

55e6360Merge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts
a86697aMerge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts

comment:17 Changed 2 years ago by git

  • Commit changed from a86697aa41c0f45abb25fd589db90bae61e57268 to 7ba1e5f4df14e81521e6a03e6bfe887612e2b56d

Branch pushed to git repo; I updated commit sha1. New commits:

7ba1e5fFix new tests in 9.1.beta9

comment:18 in reply to: ↑ 15 Changed 2 years ago by gh-mwageringel

Replying to fbissey:

My own thinking is that calling the structure dictionary lead humans to have an expectation for it to be ordered the way the physical object is. In fact we have look up tables which are very handy for the computer to manipulate but not necessarily meant for full display. In most case sorting doesn't matter objectively.

So I am not sure we should compare whole dictionaries for doctests. Size of the dictionary and some key elements should be enough.

I agree that generally doctests should be focussed on testing very specific things rather than entire dictionaries. Rewriting all these doctests case-by-case so that dictionaries are not printed anymore seems like a huge task though.

As the current patch also seems to fail with 32-bit, it still seems to me that the simplest approach to move forward with this is to extend pretty_print by a sort keyword. A first step for this is #29136, needing review, which stops pretty_print from outputting latex on the command line.

For reference, this problem was discussed in this thread on devel and there does not seem to be much disagreement about this.

comment:19 Changed 2 years ago by git

  • Commit changed from 7ba1e5f4df14e81521e6a03e6bfe887612e2b56d to 9e306eeaab6bc96bba73a496f962ff302f971995

Branch pushed to git repo; I updated commit sha1. New commits:

330ccd8Merge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts
9e306eeSync with 9.1.rc0

comment:20 Changed 2 years ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

comment:21 Changed 2 years ago by git

  • Commit changed from 9e306eeaab6bc96bba73a496f962ff302f971995 to be2afb969b72fd6db3ac7977f35b76f0df70731a

Branch pushed to git repo; I updated commit sha1. New commits:

4abe82eMerge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts
be2afb9Rebase for 9.1.rc1

comment:22 follow-up: Changed 2 years ago by mkoeppe

Is there really not a better way to do these doctest updates that can be agreed on? Using sorted(D.items()) as suggested early on this ticket seems much more reasonable

comment:23 Changed 2 years ago by git

  • Commit changed from be2afb969b72fd6db3ac7977f35b76f0df70731a to a28a1dbfd8bfd51a947a7dae78cf4e2c05bee67f

Branch pushed to git repo; I updated commit sha1. New commits:

e2980eeMerge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts
a28a1dbMerge branch 'develop' of git://git.sagemath.org/sage into t/29042/remove_sorting_of_dicts

comment:24 Changed 2 years ago by mkoeppe

  • Cc jhpalmieri dimpase added

comment:25 Changed 2 years ago by mkoeppe

  • Cc tscrim added

comment:26 in reply to: ↑ 22 Changed 2 years ago by fbissey

Replying to mkoeppe:

Is there really not a better way to do these doctest updates that can be agreed on? Using sorted(D.items()) as suggested early on this ticket seems much more reasonable

It is a quite economical way to do things. But we have to document somewhere how dictionaries have to tested going forward. As far as I can see people still want or keep testing full dictionaries. Which leaves two options

  • sorted if we are sure it does the job in every case
  • since we are typing the whole dictionary anyway, create a expected_dict populated with expected value and compare it to the result dictionary we get. I would hope dictionary comparison doesn't care about sorting.

comment:27 Changed 2 years ago by fbissey

And I think we may want to remove that one altogether

File "/usr/lib/python3.7/site-packages/sage/repl/display/pretty_print.py", line 85, in sage.repl.display.pretty_print.SagePrettyPrinter.__init__
Failed example:
    dict(zzz=123, aaa=99, xab=10)    # sorted by keys
Expected:
    {'aaa': 99, 'xab': 10, 'zzz': 123}
Got:
    {'zzz': 123, 'aaa': 99, 'xab': 10}

Which comes from

        IPython pretty printers::

            sage: set({1, 2, 3})
            {1, 2, 3}
            sage: dict(zzz=123, aaa=99, xab=10)    # sorted by keys
            {'aaa': 99, 'xab': 10, 'zzz': 123}

Pretty print no more.

comment:28 Changed 2 years ago by fbissey

I am looking at some stuff in .rst file. It is both doctests and documentation. For example in doc/en/prep/Advanced-2DPlotting.rst

Here are the options for contour plots.

- They are given as an "attribute" \- no parentheses \- of the
  ``contour_plot`` object.

- They are given as a dictionary (see :ref:`the programming tutorial
  <Advanced>`).

::

    sage: contour_plot.options
    {'aspect_ratio': 1,
     'axes': False,
     'colorbar': False,
     'contours': None,
     'fill': True,
     'frame': True,
     'labels': False,
     'legend_label': None,
     'linestyles': None,
     'linewidths': None,
     'plot_points': 100,
     'region': None}

the documentation aspect doesn't really care about the sorting. It is just displaying. But this is doctested as well which does. Using an expected_dict variable doesn't make sense in this context. If we use sorted we have to make a note about it in the documentation that it is just to make presentation consistent. If we just reproduce the output, the order may change unexpectedly.

comment:29 follow-up: Changed 2 years ago by jhpalmieri

Maybe we should just label that as "random".

comment:30 in reply to: ↑ 29 ; follow-up: Changed 2 years ago by fbissey

Replying to jhpalmieri:

Maybe we should just label that as "random".

I thought about that. Does it show in generated html/pdf?

comment:31 in reply to: ↑ 30 Changed 2 years ago by jhpalmieri

Replying to fbissey:

Replying to jhpalmieri:

Maybe we should just label that as "random".

I thought about that. Does it show in generated html/pdf?

It does. We could add a remark beforehand: The following is marked "random" because Python may print dictionaries in an unpredictable order. I think I've done this sort of thing with other doctests tagged random.

comment:32 follow-up: Changed 2 years ago by tscrim

The problem is that it also accepts something like 2 as being equal. I am -1 a strong on marking things as # random and instead replacing them with more meaningful doctests (like comparing the result to a fixed dictionary, or getting specific values in the result, etc.).

comment:33 in reply to: ↑ 32 ; follow-up: Changed 2 years ago by fbissey

Replying to tscrim:

The problem is that it also accepts something like 2 as being equal.

You are losing me here. Can you expand?

I am -1 a strong on marking things as # random and instead replacing them with more meaningful doctests (like comparing the result to a fixed dictionary, or getting specific values in the result, etc.).

We are talking about only putting random in specific documentation where it doesn't matter and ordering would look weird. But even there I would agree on getting as specific as possible to avoid having to do it. For example, in the same file as comment:28 and after it we have

sage: contour_plot.options["fill"]=False
sage: contour_plot.options
{'aspect_ratio': 1,
 'axes': False,
 'colorbar': False,
 'contours': None,
 'fill': False,
 'frame': True,
 'labels': False,
 'legend_label': None,
 'linestyles': None,
 'linewidths': None,
 'plot_points': 100,
 'region': None}

Where we could look specifically at the concerned dictionary element. But the one in comment:28 is part of an exposition of a feature. It feels ridicule to compare the output or to to artificially order it as opposed to just mentioning that the elements are not always in that order.

We have to use some common sense on what is appropriate and what isn't.

comment:34 in reply to: ↑ 33 Changed 2 years ago by tscrim

Replying to fbissey:

Replying to tscrim:

The problem is that it also accepts something like 2 as being equal.

You are losing me here. Can you expand?

If you have

sage: foo  # random
2 + 2

this also will pass even if foo is a dict or 'fish' (i.e. I can make 2 + 2 = 'fish').

I am -1 a strong on marking things as # random and instead replacing them with more meaningful doctests (like comparing the result to a fixed dictionary, or getting specific values in the result, etc.).

We are talking about only putting random in specific documentation where it doesn't matter and ordering would look weird. But even there I would agree on getting as specific as possible to avoid having to do it. For example, in the same file as comment:28

Ah, I see. I misinterpreted what you were asking about. I was thinking it was being made as a more broad statement. Sorry.

Where we could look specifically at the concerned dictionary element. But the one in comment:28 is part of an exposition of a feature. It feels ridicule to compare the output or to to artificially order it as opposed to just mentioning that the elements are not always in that order.

We have to use some common sense on what is appropriate and what isn't.

I guess in this case because it is meant to be for an illustrative purpose that it is fine for a # random with an explanation.

comment:35 Changed 2 years ago by jhpalmieri

Yes, I was only talking about this specific doctest, not all of these. I think we're in agreement, but to make it explicit: my approach is to try to understand what the doctest is testing. If it's merely illustrative, I don't mind tagging it with random. This is especially true if the same functionality is tested robustly elsewhere, although to be honest, I didn't check that in this case. Of course a random tag is no good if you're actually trying to test the output.

comment:36 follow-up: Changed 2 years ago by fbissey

It is much tougher than I expected. What do you do with cases like that?

Representatives of all divisor classes with nontrivial homology::

    sage: p = S.betti_complexes()
    sage: p[0]
    [{0: -8, 1: 5, 2: 4, 3: 1},
     Simplicial complex with vertex set (1, 2, 3) and facets {(3,), (1, 2)}]

in src/doc/en/thematic_tutorials/sandpile.rst. A list with a dictionary as an element and I don't want to destroy the exposition element.

comment:37 in reply to: ↑ 36 Changed 2 years ago by gh-mwageringel

sage: from pprint import pprint
sage: pprint(p[0])
[{0: -8, 1: 5, 2: 4, 3: 1},
 Simplicial complex with vertex set (1, 2, 3) and facets {(3,), (1, 2)}]

comment:38 Changed 2 years ago by fbissey

Yes, it is probably best here. And while they weren't apparently broken by ipython >= 7.10 there are other instances in that file that probably should be pprint-ed.

comment:39 Changed 2 years ago by fbissey

  • Branch changed from u/arojas/remove_sorting_of_dicts to u/fbissey/dictionaries
  • Commit changed from a28a1dbfd8bfd51a947a7dae78cf4e2c05bee67f to 2594a80ff25b466400b2f117b21d48b1aff73879

Attaching a WIP branch for people to comment on. I tried to use an appropriate method depending on the context. The branch focuses on the .rst files under src/doc with the idea that they may present a good sample of issues.


New commits:

016d82fIn this tutorial, we can precisely follow the insertion order of the dictionaries.
2594a80Deal with dictionaries with pprint, sorted or other as appropriate.

comment:40 Changed 2 years ago by arojas

  • Cc arojas added

comment:41 Changed 2 years ago by fbissey

I have not as much time to deal with this as I thought. If someone wants to take over or go back to arojas' branch, it is fine by me.

comment:42 Changed 2 years ago by arojas

I am maintaining the patch downstream anyway, so I can push it here for other packagers to use it until someone makes a decision on this (I'm not planning to work on implementing a different solution myself)

comment:43 Changed 2 years ago by arojas

Also, I don't see the need to give a final solution to this *now*. We could just merge my patch to unblock the ipython upgrade, and keep this open to implement a different solution when someone has time/willingness to work on it.

EDIT: Actually, I forgot about 32-bit, so it seems the patch can't be merged as-is anyway

Last edited 2 years ago by arojas (previous) (diff)

comment:44 Changed 2 years ago by fbissey

Darn 32-bit. I otherwise agree with you. We can make this better at a later stage.

comment:45 follow-up: Changed 2 years ago by dimpase

I don't quite understand what this ticket is about: This will break the affected doctests (but not functionality) on python2

9.2 has nothing to do with python2. We can break it, why not?

comment:46 in reply to: ↑ 45 Changed 2 years ago by arojas

Replying to dimpase:

I don't quite understand what this ticket is about: This will break the affected doctests (but not functionality) on python2

9.2 has nothing to do with python2. We can break it, why not?

This was opened in January

comment:47 Changed 2 years ago by dimpase

is anything needed here for py3 ?

comment:48 Changed 2 years ago by arojas

Yes - this is meant to fix the breakage that will happen in tests when upgrading ipython. "This will break the affected doctests (but not functionality) on python2" refers to the proposed patch at the time.

comment:49 Changed 2 years ago by dimpase

Could someone knowledgeable about the ticket please amend its description?

comment:50 Changed 2 years ago by arojas

  • Description modified (diff)

comment:51 Changed 2 years ago by gh-kliem

Maybe removing the sorting of dictionaries isn't necessary. We can just overwrite the pretty print function during testing:

+++ b/src/sage/doctest/forker.py
@@ -61,6 +61,7 @@ from .parsing import SageOutputChecker, pre_hash, get_source
 from sage.repl.user_globals import set_globals
 from sage.cpython.atexit import restore_atexit
 from sage.cpython.string import bytes_to_str, str_to_bytes
+import IPython.lib.pretty
 
 
 # All doctests run as if the following future imports are present
@@ -86,6 +87,27 @@ _OSError_SUBCLASSES = [
        exc is not OSError
 ]
 
+def _sorted_dict_pprinter_factory(start, end):
+    """
+    Factory that returns a pprint function used by the default pprint of
+    dicts and dict proxies.
+    """
+    def inner(obj, p, cycle):
+        if cycle:
+            return p.text('{...}')
+        step = len(start)
+        p.begin_group(step, start)
+        keys = obj.keys()
+        keys = IPython.lib.pretty._sorted_for_pprint(keys)
+        for idx, key in p._enumerate(keys):
+            if idx:
+                p.text(',')
+                p.breakable()
+            p.pretty(key)
+            p.text(': ')
+            p.pretty(obj[key])
+        p.end_group(step, end)
+    return inner
 
 
 def init_sage():
@@ -185,11 +207,11 @@ def init_sage():
     # IPython's pretty printer sorts the repr of dicts by their keys by default
     # (or their keys' str() if they are not otherwise orderable).  However, it
     # disables this for CPython 3.6+ opting to instead display dicts' "natural"
-    # insertion order, which is preserved in those versions).  This makes for
-    # inconsistent results with Python 2 tests that return dicts, so here we
-    # force the Python 2 style dict printing
-    import IPython.lib.pretty
-    IPython.lib.pretty.DICT_IS_ORDERED = False
+    # insertion order, which is preserved in those versions).
+    # However, this order is random in some instances.
+    # Also code changes may change the order, but the result stays correct.
+    # So we force sorting the dictionary.
+    IPython.lib.pretty.for_type(dict, _sorted_dict_pprinter_factory('{', '}'))
 
     # Switch on extra debugging
     from sage.structure.debug_options import debug

This appears to do the job. I'm testing the entire library now.

comment:52 Changed 2 years ago by gh-kliem

  • Authors changed from Antonio Rojas to Jonathan Kliem, Antonio Rojas
  • Branch changed from u/fbissey/dictionaries to public/29042
  • Commit changed from 2594a80ff25b466400b2f117b21d48b1aff73879 to 68b10e1e6665892edad775e6b9b18e93afc3d637
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Remove sorting of dicts to Sort dictionaries for doctests independent of ipythons pretty print

sage -t --long --all passes for me with this branch.


New commits:

68b10e1keep dictionaries ordered for doctesting

comment:53 Changed 2 years ago by fbissey

I can do some testing tomorrow. It looks so much simpler.

comment:54 Changed 2 years ago by gh-kliem

I was suprised how easy this actually is. Took me a number of hours to figure out though.

comment:55 Changed 2 years ago by dimpase

Does this need new iPython?

comment:56 Changed 2 years ago by gh-kliem

No, I didn't upgrade and it worked for me.

The function _sorted_for_pprint that it uses requires iPython at least 5:

https://github.com/ipython/ipython/blob/5.x/IPython/lib/pretty.py

If that is a problem or if this is ever removed, we can just copy paste this tiny function.

The function for_type has been in their since IPython version 1. So I hope that it is somewhat stable.

comment:57 Changed 2 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Works for me. That's one of the things I would like to see merged soon.

comment:58 Changed 2 years ago by gh-kliem

Thank you.

comment:59 Changed 2 years ago by dimpase

  • Priority changed from major to critical

hopefully it simplifies the thankless task of allowing multiple versions of pari/gp, etc.

comment:60 Changed 2 years ago by vbraun

  • Branch changed from public/29042 to 68b10e1e6665892edad775e6b9b18e93afc3d637
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.