Skip to content

Conversation

@dangra
Copy link
Contributor

@dangra dangra commented Jun 5, 2019

[WIP] Opened to measure acceptance by maintainers.
Tests and docs will be updated once API is agreed (if any)

Description

This change aims to avoid the boilerplate of applying a JsonPath to a document

>>> import jsonpath2 as jsonpath
>>> doc = {'hello': 'Hello, world!'}
>>> [m.current_value for m in jsonpath.match('$["hello"]', doc)]

It resembles the use of re module in re.match(pattern, string) vs re.compile(pattern).match(string)

Issues Resolved

None.

Check List

  • All tests pass.
  • New functionality includes testing.
  • New functionality has been documented in the README if applicable

@dmlb2000
Copy link
Member

dmlb2000 commented Jun 5, 2019

@dangra I think the thought behind this is fine, please make sure the pipeline passes.

@dmlb2000
Copy link
Member

dmlb2000 commented Jun 9, 2019

@dangra can you make a test to cover the code? either add the call to

def test_example(self):
or make another method and call your match function?

@dangra
Copy link
Contributor Author

dangra commented Jun 9, 2019 via email

dmlb2000
dmlb2000 previously approved these changes Jun 9, 2019
Copy link
Member

@dmlb2000 dmlb2000 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dmlb2000
Copy link
Member

dmlb2000 commented Jun 9, 2019

@dangra Forgot to add a bit to the README to document the new method. Could you add that?

dangra added 4 commits June 9, 2019 18:08
This change aims to avoid the boilerplate of applying a JsonPath to a document

```python
>>> import jsonpath2 as jsonpath
>>> doc = {'hello': 'Hello, world!'}
>>> [m.current_value for m in jsonpath.match('$["hello"]', doc)]
```

It resembles the use of `re` module in `re.match(pattern, string)` vs `re.compile(pattern).match(string)`
@dangra
Copy link
Contributor Author

dangra commented Jun 9, 2019

@dmlb2000 I think I broke your accepted review by force pushing a rebase to cleanup commit history. Feel free to review again and merge. thanks

Copy link
Member

@dmlb2000 dmlb2000 left a comment

Choose a reason for hiding this comment

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

@dangra looks good to me, should be able to merge and pull a release out of it soon. I'll want to setup the readthedocs integration and move some files around before that but this will be cut as a release soon.

@dmlb2000 dmlb2000 merged commit 47bd5f6 into pacifica:master Jun 9, 2019
@dangra dangra deleted the patch-2 branch June 10, 2019 01:14
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.

2 participants