Support JMESPath now #181
Support JMESPath now #181
Conversation
Support jpath based on JMESPath, It can make extract more efficient, especially on Json mixed HTML
Codecov Report
@@ Coverage Diff @@
## master #181 +/- ##
==========================================
- Coverage 91.69% 90.54% -1.15%
==========================================
Files 5 5
Lines 349 455 +106
Branches 69 93 +24
==========================================
+ Hits 320 412 +92
- Misses 23 32 +9
- Partials 6 11 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
fix unicode error
fix support 2.7
fix: flake8 problem
fix: Jpath Error
EchoShoot
changed the title
|
Do you think you can include tests that cover the lines marked in yellow/red in https://codecov.io/gh/scrapy/parsel/pull/181/diff? |
|
@Gallaecio Thank you for your continued guidance, I will fix the problem on the weekend, sorry for the late reply |
|
@EchoShoot Do you plan to get back to this? Otherwise, would you mind if I take over? |
I have been too busy working lately, and I ca n’t find time to do it. I will be happy to entrust it to you. |
Co-authored-by: Andrey Rakhmatullin <wrar@wrar.name>
|
|
||
| def _is_valid_json(text: str) -> bool: | ||
| try: | ||
| json.loads(text) |
There was a problem hiding this comment.
How bad is it to run json.loads an extra time in case of big files?
There was a problem hiding this comment.
I think the real question isnt on the performance hit of the operation, but "is there any other way of verifying something is a valid json that doesn't entail loading it?"
Any other approach here would require a significant refactoring job, which could by itself be an enhancement of the feature in another PR/Issue down the line.
There was a problem hiding this comment.
Yeah, I don't think there are such ways.
|
Thank you everyone! |
|
|
|
Great job @EchoShoot @Gallaecio @felipeboffnunes! |
Support jpath based on JMESPath, It can make extract more efficient, especially on Json mixed HTML, and i write test file too, your could test it!
To-do:
Post-merge work:
Closes #25, #264