search

jmespath / jmespath.py

JMESPath is a query language for JSON.
http://jmespath.org
MIT License
2.19k stars 181 forks source link

Jep 3 #43

Closed jamesls closed 10 years ago

jamesls commented 10 years ago

Implement JEP-3, latest spec is https://github.com/jamesls/jmespath/blob/mtdowling-functions/docs/proposals/functions.rst

I need to update the specification.rst with the docs from the JEP linked above, but I'd like to kick off the code review now.

cc @toastdriven @mtdowling @danielgtaylor

toastdriven commented 10 years ago

Outside those questions, LGTM.

jamesls commented 10 years ago

Looking into the failing tests (not seeing them on my end so I'm not sure what's going on).

Also, based on some discussions, it seems like sort_by is not quite ready to go in. We're missing some concept of an expression/function type. For example, in python, sort would take a key argument that is a function. To be consistent, we would need something similar here instead of as I have where the docs just state that it should not be resolved.

I want to look into this ASAP, but for now, I'm going to pull sort_by and defer to a later JEP. We can likely add a number of *_by functions later (sort_by, max_by, min_by, map, fold, etc.) that use this concept.

toastdriven commented 10 years ago

The new commits look good as well.

danielgtaylor commented 10 years ago

This generally looks good to me as well. Nice job on the tests. I'd suggest maybe adding some small examples of why you would use these to the spec, just for illustrative purposes before diving into how to use them.

jamesls commented 10 years ago

Mind taking another look? I think I've updated all feedback

toastdriven commented 10 years ago

:ship: it!