tommikaikkonen / prettyprinter

Syntax-highlighting, declarative and composable pretty printer for Python 3.5+
https://prettyprinter.readthedocs.io
MIT License
336 stars 20 forks source link

Stop sorting dicts (at least by default)? #5

Closed njsmith closed 6 years ago

njsmith commented 6 years ago

Traditionally, pprint has sorted dict keys, because dict order was arbitrary so we might as well arbitrarily pick a nice order. But starting in 3.6 unofficially, and 3.7 officially, all dicts are effectively OrderedDicts (modulo some details about how == works and a few extra methods that OrderedDict has). So probably the printer should show this order.

tommikaikkonen commented 6 years ago

Agreed. Further, if people start leaning heavily on the ordered property of dicts, sorting the keys could be actively misleading. Like you mentioned, users may want to override this when using a dict as a real dictionary, e.g. when printing a mapping of 100+ hostnames to IP addresses.

Thinking about a easy interface to do that... We could let the user control that in prettyprinter.prettyprinter.PrettyContext, an instance of which is passed to every pretty printer definition. As an example, if they have a DNSLookupDict class that wraps a native dict mapping hostnames to IP addresses that they want to sort when pretty printing, they could use ctx.set(DictOrdering, DictOrdering.SORTED) in their pretty printer definition. Something like this:

# In the prettyprinter module...
class DictOrdering(enum.Enum):
    INSERTION = 0
    SORTED = 1

# user-land
from prettyprinter import DictOrdering, register_pretty, pretty_call

class DNSLookupDict:
    ...

@register_pretty(DNSLookupDict)
def pretty_dnslookupdict(d, ctx):
    return pretty_call(
        ctx.set(DictOrdering, DictOrdering.SORTED),
        DNSLookupDict,
        dict(d)
    )

To get an output like

DNSLookupDict({
    'a.com': '127.0.0.2',
    'b.com': '127.0.0.1',
    'c.com': '127.0.0.3',
    'd.com': '127.0.0.4',
})

It could also be first class in PrettyContext, e.g.

ctx.sort_dicts(True)

but I feel like sticking to the simple ctx.set(k, v) would keep the API surface smaller, even if the user needs to additionally import DictOrdering. I'm considering adding a public verbosity context setting in the future, similar to what I do in the Django definitions, which would also work like ctx.set(Verbosity, Verbosity.MINIMAL)

Overriding the context is not as straightforward at the top level calls though, e.g. prettyprinter.cpprint, because the initial PrettyContext instance is created based on the arguments to that function. An additional keyword-only argument such as sort_dicts: bool could work, so that users may do cpprint(my_dict, sort_dicts=True) without having to import anything additional. It's not very consistent with the ctx.set(DictOrdering, DictOrdering.SORTED) interface, though.

Unless anyone chimes in with better ideas, I'll take some time during the weekend to implement this using the ctx.set(DictOrdering, DictOrdering.SORTED) interface and default it to DictOrdering.INSERTION.

njsmith commented 6 years ago

do you want a mutator (set), or something that returns a copy of the of the context with different settings?

njsmith commented 6 years ago

Oh, then I clicked through to the definition of PrettyContext, and discovered that set is already a return-a-copy interface, not a mutator interface. Well, I guess that's a vote for thinking about renaming it :-)

tommikaikkonen commented 6 years ago

Yep, PrettyContext is fully immutable except for the visited attribute, which is a mutable set tracking visited values to prevent recursion. Having that mutable is just a performance optimization. So PrettyContext.set is indeed misleading; perhaps replace would be Pythonic as used in the immutable APIs for datetime.datetime and namedtuple._replace.

tommikaikkonen commented 6 years ago

This is fixed in 0.8.0 that I just released on PyPi. Dict key sorting can be controlled in the printing functions pprint, cpprint, and pformat with the sort_dict_keys keyword argument. A global default can be set with prettyprinter.set_default_config(sort_dict_keys=True). It can also be controlled from PrettyContext, though the API for that uses the not-explicitly-public _replace method. The class needs more documentation and thought to make it fully public.

njsmith commented 6 years ago

awesome :-)

On Sun, Dec 31, 2017 at 2:16 PM, Tommi Kaikkonen notifications@github.com wrote:

Closed #5 https://github.com/tommikaikkonen/prettyprinter/issues/5.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tommikaikkonen/prettyprinter/issues/5#event-1405422629, or mute the thread https://github.com/notifications/unsubscribe-auth/AAlOaBg8yCUfthjN2jv_CK_eY_ShEaaeks5tGAe1gaJpZM4RO0G7 .

-- Nathaniel J. Smith -- https://vorpus.org http://vorpus.org