search

Averroes / simplejson

MIT License
0 stars 0 forks source link

encoder: allow passing in an arbitrary sort cmp function #33

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Here's a patch against the current trunk that allows you to pass in your
own cmp function.  This is useful in situations where an alphabetical
sorting is
not desired and thus sort_keys=True is not sufficient.

Original issue reported on code.google.com by dav...@gmail.com on 6 Jan 2009 at 8:42

Attachments:

GoogleCodeExporter commented 9 years ago 
I'm going to refuse this patch, I'm not clear why this is useful. Since the 
ordering is irrelevant to decoding, sort_keys=True is only useful for debugging 
purposes, so the maintenance effort of supporting this seems a bit 
high to support extra documentation, tests, etc.

Original comment by bob.ippo...@gmail.com on 6 Jan 2009 at 10:14

GoogleCodeExporter commented 9 years ago 
This is useful when humans have to edit machine-generated JSON by hand.

It's not a debugging thing -- this is to improve usability for fine, grand 
users. 
One of JSON's main marketing points is that it is 'user-friendly.'

True, the ordering *is* irrelevant to the decoder, but it is extremely relevant 
to
the human sitting inbetween the encoder and decoder.

Some people actually edit JSON by hand.  This patch allows us to provide much 
finer
control over the presentation of the content provided to them.

Is that a good enough justification?  If not, thanks nonetheless.

Original comment by dav...@gmail.com on 6 Jan 2009 at 10:33

GoogleCodeExporter commented 9 years ago 
I'll reconsider if you submit a patch that includes test coverage and 
documentation

Original comment by bob.ippo...@gmail.com on 6 Jan 2009 at 4:15

GoogleCodeExporter commented 9 years ago 
Thank you!

Here's the new patch series:

0001 - this adds the cmp_func flag to dump().
0002 - this adds test coverage (I tested it with nose)
0003 - this adds documentation

I'm not sure if I did the documentation correctly since it seems like the same
documentation existed in two places.  I may have been editing an auto-generated 
file.
 In any case, let me know if there's anything you'd like me to fixup and I'll go
ahead and resubmit.

The original patch used "cmp" as the kwarg but I changed it to cmp_func since 
it's
not good practice to use variables that share the same name as Python builtins.

Thanks for your consideration,
-David

Original comment by dav...@gmail.com on 6 Jan 2009 at 11:21

Attachments:

GoogleCodeExporter commented 9 years ago 
I think that cmp is probably more appropriate, even though it does get in the 
way of PEP8 guidelines.

sort(...)
    L.sort(cmp=None, key=None, reverse=False) -- stable sort *IN PLACE*;
    cmp(x, y) -> -1, 0, 1

sorted(...)
    sorted(iterable, cmp=None, key=None, reverse=False) --> new sorted list

Original comment by bob.ippo...@gmail.com on 6 Jan 2009 at 11:29

GoogleCodeExporter commented 9 years ago

Original comment by bob.ippo...@gmail.com on 6 Jan 2009 at 11:30

GoogleCodeExporter commented 9 years ago 
Should I resend 0001 or can you fix it up on your end?

Original comment by dav...@gmail.com on 6 Jan 2009 at 11:47

GoogleCodeExporter commented 9 years ago 
I'll go ahead and regex it on my end when I take a closer look at the patches

Original comment by bob.ippo...@gmail.com on 6 Jan 2009 at 11:52

GoogleCodeExporter commented 9 years ago 
I realized that there was a typo ("if" vs "If") in the documentation patch so I 
went
ahead and rebased my changes to use "cmp" instead of "cmp_func".

Thanks again,
-David

Original comment by dav...@gmail.com on 7 Jan 2009 at 12:42

Attachments:

GoogleCodeExporter commented 9 years ago 
I'm going to hold off on this until simplejson 2.1 because it's a feature 
request and not a bug fix, so it's not 
appropriate for merging with Python 2.6/Python 3. I've attached a unified diff 
for bringing this functionality in.

Original comment by bob.ippo...@gmail.com on 16 Feb 2009 at 12:31

Attachments:

GoogleCodeExporter commented 9 years ago 
There is a similar patch going into the Python 2.7/3.1 branch of json that uses 
an insertion-order dictionary to 
keep track of the original key order in the file. Would having this feature 
eliminate the use case for this cmp 
function? I would include an ordered dict implementation in simplejson to make 
it easy for users of older Python 
versions.

Original comment by bob.ippo...@gmail.com on 28 Mar 2009 at 5:01

GoogleCodeExporter commented 9 years ago 
Yes, maintaining the order in the file (and having an ordered dict for new-file
creation) would eliminate this use case.

Original comment by dav...@gmail.com on 28 Mar 2009 at 8:43

GoogleCodeExporter commented 9 years ago 
r178 allows order to be optionally maintained via object_pairs_hook, there's a 
demonstration in simplejson.tool, 
basically just loads(s, object_pairs_hook=simplejson.OrderedDict)

The repr of OrderedDict isn't exactly like dict but you could change that in 
your own OrderedDict subclass.

Original comment by bob.ippo...@gmail.com on 29 Mar 2009 at 9:41

GoogleCodeExporter commented 9 years ago 
I would like to add that ordering is also important for a very non-debugging 
purpose: to be able to get two serializations of the dame data to be binary 
equivalent. This means that you can do stuff like verify if the data changed 
w/o decoding/encoding it.

        cookie_str = request.get_cookie()
        cookie = simplejson.loads(cookie_str)
        res = fn(request, cookie=cookie, *arg, **kw)
        if res is None:
            res = {}
        new_cookie_str = simplejson.dumps(cookie, default=jsonify_models)
        if new_cookie_str != cookie_str:
            response.set_cookie(new_cookie_str)

Original comment by redhog....@gmail.com on 20 Oct 2010 at 2:52

GoogleCodeExporter commented 9 years ago 
Yes, it is an important optimization to have a canonical representation, but 
you can do that with sort_keys as-is. What are you trying to ask here? This 
ticket is closed. Open a new one if you want something new.

Original comment by bob.ippo...@gmail.com on 20 Oct 2010 at 3:08