Skip to content

Conversation

@sileht
Copy link
Contributor

@sileht sileht commented Aug 4, 2015

This extends the jsonpath_rw parser with customisation
for gabbi.

This first one is len

@sileht sileht force-pushed the sileht/add-len branch 2 times, most recently from 57163bf to fc06658 Compare August 4, 2015 16:34
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it should, but does it make any sense for it also to work on str and dict?

In fact, would it make any sense to just call len and handle TypeError if the datum.value can't len()?

I can certainly imagine situations where people will want to know the length of a dict that is being returned.

What do you think?

Bringing in @FND as he may have an opinion too. FND, in case you feel like this is out of the blue, especially with regard to discussions we've had about dealing with location, there's some precedent, I've been looking for months to add sorts but hadn't figured out the best way to integrate. This gives us a place to hang such things. If we choose to do so.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the brains to parse the code in detail right now or think deeply about the conceptual implications. However your suggestion WRT type agnosticism makes sense. In fact, I can imagine users simply assume that it works for any iterable, so not supporting that would likely end up being surprising.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I make it generic and handle TypeError, str len is clearly a must have.

This extends the jsonpath_rw parser with customisation
for gabbi.

This first one is `len`
@cdent
Copy link
Owner

cdent commented Aug 4, 2015

There was some irc-based discussion about the possibility of this being a module of its own, an independent package of jsonpath improvements that gabbi happens to use. I think there's merit in that idea but for now we'll go ahead and merge this and consider it as an option for the future.

cdent added a commit that referenced this pull request Aug 4, 2015
Add `len` extention to the json parser
@cdent cdent merged commit 69c5e07 into cdent:master Aug 4, 2015
@sileht sileht deleted the sileht/add-len branch August 5, 2015 09:57
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.

3 participants