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 update method on most JSONPath descendents. Tests included. #28

Merged
merged 1 commit into from
Oct 5, 2016

Conversation

joshbenner
Copy link
Contributor

Mostly addresses #21. I didn't add update() implementations for Parent, Union, or Intersect, as I wasn't sure it made sense to do so. More esoteric test cases might be good, too, as I'm not 100% sure the implementations work as expected at all times.

@mgcdanny
Copy link

mgcdanny commented Aug 4, 2015

Is there a reason why this hasn't been merged in to the master branch?

@mgcdanny
Copy link

mgcdanny commented Aug 4, 2015

@joshbenner
Either I am doing something wrong or I found a bug with this pull request. Check out this example:

from jsonpath_rw import parse
jsonpath_expr = parse('$..baz')
data = {'foo': [{'baz': 1}, {'baz': 2}]}
jsonpath_expr.update(data, '1234')
assert data == {'foo': [{'baz': 1234}, {'baz': 1234}]}

@kennknowles
Copy link
Owner

At a high level, I'd kind of like to reserve the word update for creating a copy with new values in certain places, and call this set, but I like this. It was even my original intent with the library, but I got too busy. I will be happy with whatever name is more Pythonic.

@kennknowles
Copy link
Owner

More tests, such as those with lists within dictionaries and vice versa, would be nice.

@ailjushkin
Copy link
Collaborator

I believe it is very handy PR, can anyone tell me what is needed to merge it?

@jonathanglima
Copy link

+1

this will also update passing lambdas instead of the value? if not, that's a great enhancement

@ailjushkin
Copy link
Collaborator

Should we believe this PR could be aprooved?

@carpnick
Copy link

+1

@cliffpracht
Copy link

+1. Useful feature.

@carlosmmelo
Copy link

+1

@kennknowles kennknowles merged commit 1235def into kennknowles:master Oct 5, 2016
kennknowles added a commit that referenced this pull request Oct 5, 2016
@quazar0
Copy link

quazar0 commented Dec 29, 2016

Thanks for this update. BTW, does this allow removal of a selected element?

@shuaiyuancn
Copy link

I'm seeing NotImplementedError:

from jsonpath_rw.parser import parse
parse('$.a').update({'a': 1}, 2)
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<ipython-input-59-940582069d38> in <module>()
      1 from jsonpath_rw.parser import parse
----> 2 parse('$.a').update({'a': 1}, 2)

/databricks/python/local/lib/python2.7/site-packages/jsonpath_rw/jsonpath.pyc in update(self, data, val)
     28     def update(self, data, val):
     29         "Returns `data` with the specified path replaced by `val`"
---> 30         raise NotImplementedError()
     31 
     32     def child(self, child):

NotImplementedError: 

@GusBricker
Copy link

The reason you are getting the NotImplementedError is because the current version (1.4.0) on PyPi predates this pull request.

@namratha-bharadwaj
Copy link

Hello... I see the "raise NotImplementedError(), NotImplementedError" when I tried using the update method to change json elements. Is there a work-around for this?

@srhopkins
Copy link

Are the plans to cut a new release soon with the update feature?

@kennknowles
Copy link
Owner

It has been so long since I did a pypi release I don't remember how ;_;

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.