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

Support JSONPath #237

Closed
wants to merge 5 commits into from
Closed

Support JSONPath #237

wants to merge 5 commits into from

Conversation

deepakdinesh1123
Copy link
Contributor

Support for JSONPath has been added with the jsonpath-ng library. Most of the implementation is based on #181 which adds support for json using the JMESPath library.

closes #204

@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #237 (302b80b) into master (f5f73d3) will decrease coverage by 0.61%.
The diff coverage is 96.77%.

@@             Coverage Diff             @@
##            master     #237      +/-   ##
===========================================
- Coverage   100.00%   99.38%   -0.62%     
===========================================
  Files            5        5              
  Lines          290      327      +37     
  Branches        59       72      +13     
===========================================
+ Hits           290      325      +35     
- Misses           0        1       +1     
- Partials         0        1       +1     
Impacted Files Coverage Δ
parsel/selector.py 98.94% <96.55%> (-1.06%) ⬇️
parsel/csstranslator.py 100.00% <100.00%> (ø)
parsel/xpathfuncs.py 100.00% <100.00%> (ø)

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 f5f73d3...302b80b. Read the comment docs.

@deepakdinesh1123
Copy link
Contributor Author

I have included jsonpath_ng in setup.py but flake8 is throwing an error saying that jsonpath_ng not found. How do I resolve it?

@deepakdinesh1123 deepakdinesh1123 marked this pull request as ready for review March 16, 2022 07:36
@Gallaecio
Copy link
Member

The flake errors, if you look at lines 71-74 of the CI output, is:

/home/runner/work/parsel/parsel/docs/conf.py:87:5: E128 continuation line under-indented for visual indent
/home/runner/work/parsel/parsel/docs/conf.py:88:5: E128 continuation line under-indented for visual indent
/home/runner/work/parsel/parsel/docs/conf.py:98:5: E128 continuation line under-indented for visual indent
/home/runner/work/parsel/parsel/docs/conf.py:99:5: E128 continuation line under-indented for visual indent

I suggest you run black docs/conf.py to address it. And it would be great if you could modify https://github.com/scrapy/parsel/blob/master/tox.ini#L42 to include the docs folder among the black default arguments.

@deepakdinesh1123
Copy link
Contributor Author

After reformatting docs/conf.py using black, flake8 is failing with line too long error, I was not able to catch this error while running tox locally, as flake8 fails with the following error ERROR - ModuleNotFoundError: No module named 'jsonpath_ng'.

@Gallaecio
Copy link
Member

I believe you need to run black setting the line limit to 79 to comply with flake8:

black --line-length=79 …

We probably did not hit this issue in the rest of the code by pure luck, but we need to edit tox.ini to specify that length as well, otherwise later executions of black will break the length again.

As for the module error when running tox, since it works in CI, my guess is that you need to recreate your Tox environment, i.e. include --recreate in your tox command once per environment (or just once if you run all environments at once).

@deepakdinesh1123 deepakdinesh1123 closed this by deleting the head repository Feb 17, 2024
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.

Add JSONPath support
2 participants