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

Try to fix bad JSON due to unescaped double quotes #126

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Gallaecio
Copy link
Member

Fixes #53

If we merge this, we should create a separate issue to handle #53 (comment), which probably requires a custom fallback JSON parser.

@codecov
Copy link

codecov bot commented May 7, 2020

Codecov Report

Merging #126 (96bf6b3) into master (d78167c) will increase coverage by 0.28%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #126      +/-   ##
==========================================
+ Coverage   90.24%   90.52%   +0.28%     
==========================================
  Files          13       13              
  Lines         605      623      +18     
  Branches      136      137       +1     
==========================================
+ Hits          546      564      +18     
  Misses         52       52              
  Partials        7        7              
Impacted Files Coverage Δ
extruct/jsonld.py 100.00% <100.00%> (ø)
extruct/rdfa.py 100.00% <100.00%> (ø)
extruct/utils.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 d78167c...96bf6b3. Read the comment docs.

@whalebot-helmsman
Copy link

Just found https://github.com/codecobblers/dirtyjson by fast googling. May be it have sense to use it here.

@Gallaecio
Copy link
Member Author

Using a library would definitely be better.

I’m a bit worried about the library not having received changes since 2017, and @scottkmaxwell not being active in GitHub since 2018.

On the other hand, there are no open issues or pull requests in the repository, and worse case scenario we could fork the library.

I’ll give it a try.

@Gallaecio
Copy link
Member Author

According to #137 (comment) it won’t work.

@whalebot-helmsman
Copy link

@Kiollpt thanks for checking and @Gallaecio thanks for passing the message

@scottkmaxwell
Copy link

Using a library would definitely be better.

I’m a bit worried about the library not having received changes since 2017, and @scottkmaxwell not being active in GitHub since 2018.

On the other hand, there are no open issues or pull requests in the repository, and worse case scenario we could fork the library.

I’ll give it a try.

I'm still around, just busy with my day job. It looks like I am not getting my GitHub notifications in email so I'll fix that now. Feel free to use or fork. If you want to just use dirtyjson, I'll try to be responsive to PRs.

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.

How to correct "nasty" jsonl+ld
3 participants