Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implemented items, to_object and zip functions #105

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

jstewmon
Copy link

implements jmespath/jmespath.jep#20

I think to_object may need additional input checking, but I'm not sure what the behavior should be for some error cases. For example, to_object([[1, 2, 3]]) will raise ValueError: dictionary update sequence element #0 has length 3; 2 is required. And to_object([[[1], 1]]) will raise TypeError: unhashable type: 'list'.

Does the spec prescribe a behavior (like a specific error type) for these cases?

@jamesls
Copy link
Member

jamesls commented Mar 21, 2016

Awesome, thanks for the pull request.

I would expect both of those cases to be errors, potentially type errors. I'm not sure if it's worth updating the type checker to support some sort of record/tuple type (that would just be a list with a fixed structure) or just adding this check in to_object. Something like to_object(array[tuple[str, any]]). I'd be ok with either for now.

As an aside, I noticed the various to_* functions in JMESPath will take any type and have specific behavior defined for each input type. Given this is the first to_* function that doesn't follow this pattern, it might make sense to name this function something different.

@jstewmon
Copy link
Author

Regarding the naming convention of to_* functions, would it be reasonable to have to_object return null for inputs that cannot be represented as an object? That would seem to be congruent with the to_number spec, especially if the type checker supports a tuple type because the spec could clearly indicate that any other type results in null. number and object are similar in that there is a limited set of types from which a cast is possible.

That said, as a user I would find a type error is more informative than null. Does to_number result in null instead of a type error because the expectation of all to_* functions is to return either a casted representation of the input or null?

@codecov-io
Copy link

codecov-io commented Mar 21, 2016

Codecov Report

Merging #105 into develop will decrease coverage by 0.49%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop     jmespath/jmespath.site#105     +/-   ##
==========================================
- Coverage    97.27%   96.77%   -0.5%     
==========================================
  Files           13       13             
  Lines         1431     1459     +28     
==========================================
+ Hits          1392     1412     +20     
- Misses          39       47      +8
Impacted Files Coverage Δ
jmespath/functions.py 99.51% <100%> (+0.02%) ⬆️
jmespath/compat.py 79.66% <100%> (-10.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef36ae7...bc92aae. Read the comment docs.

@jamesls
Copy link
Member

jamesls commented Mar 23, 2016

The main reason to_number defined nulls instead of type errors was so you could use it as a succinct way to filter out non-numeric values when combined with a projection. For example:

>>> jmespath.search('sum([*].a.to_number(@))', [{"a": []}, {"a": "1"}, {"a": 2}, {"a": None}])
3

Those conditions don't really apply with to_object because it's not intended to be a generic conversion function. It expects a specific structure in its input arguments, and will error out if it doesn't get the structure it expects. That's why I'm hesitant for the name to suggest it's a generic function. I feel something like from_pairs would clarify its expectations more explicitly. What do you think?

@jstewmon
Copy link
Author

jstewmon commented Apr 2, 2016

Ok, I see your point. I think either from_pairs or from_items are good names. I like the latter because it expresses that this function is the complement of items.

Do you like from_items, or do you prefer from_pairs?

@jstewmon
Copy link
Author

jstewmon commented May 5, 2017

@jamesls I had forgotten about this PR, but someone just mentioned to me that it is linked in a StackOverflow answer, so I thought I'd ping you to see if you are still interested in accepting this PR and ask if you have any more feedback?

I rebased on the latest develop branch.

@curtisforrester
Copy link

Bump. This might have been nice for a use-case yesterday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants