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.

Sign up for GitHub

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

Jump to bottom

[REVIEW] Handle double quote and escape character in json #6296

Merged
merged 17 commits into from
Sep 25, 2020
Merged

[REVIEW] Handle double quote and escape character in json #6296

merged 17 commits into from
Sep 25, 2020

Conversation

rgsl888prabhu
Copy link
Contributor

closes #6270

@rgsl888prabhu rgsl888prabhu added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. 4 - Needs cuDF (Python) Reviewer 4 - Needs Review Waiting for reviewer to review or respond cuIO cuIO issue labels Sep 22, 2020
@rgsl888prabhu rgsl888prabhu requested review from a team as code owners September 22, 2020 19:49
@rgsl888prabhu rgsl888prabhu self-assigned this Sep 22, 2020
@GPUtester
Copy link
Collaborator

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

cwharris
cwharris requested changes Sep 22, 2020
View reviewed changes
cpp/src/io/utilities/parsing_utils.cuh Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_json.py Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
davidwendt
davidwendt requested changes Sep 22, 2020
View reviewed changes
Copy link
Contributor

@davidwendt davidwendt left a comment

Choose a reason for hiding this comment

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

Just some comments to be fixed

cpp/include/cudf/strings/detail/replace.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/strings/detail/replace.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/strings/detail/replace.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/strings/detail/replace.hpp Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 22, 2020 •
edited
Loading

Codecov Report

Merging #6296 into branch-0.16 will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.16    #6296      +/-   ##
===============================================
+ Coverage        83.16%   83.18%   +0.02%     
===============================================
  Files               90       90              
  Lines            14692    14693       +1     
===============================================
+ Hits             12218    12222       +4     
+ Misses            2474     2471       -3
Impacted Files Coverage Δ
python/cudf/cudf/testing/fuzzer.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/abc.py 91.48% <0.00%> (+4.25%) ⬆️
python/cudf/cudf/utils/gpu_utils.py 58.53% <0.00%> (+4.87%) ⬆️

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 5f62462...5a0276b. Read the comment docs.

kkraus14
kkraus14 approved these changes Sep 23, 2020
View reviewed changes
Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

pytest lgtm

@rgsl888prabhu
Copy link
Contributor Author

@cwharris @vuule

@rgsl888prabhu rgsl888prabhu merged commit ffd0c61 into rapidsai:branch-0.16 Sep 25, 2020
cwharris pushed a commit to cwharris/cudf that referenced this pull request Oct 2, 2020
@rgsl888prabhu @davidwendt @cwharris
[REVIEW] Handle double quote and escape character in json (rapidsai#6296
ca6043f 
)

* Change to handle escape char in case of json

* CHANGELOG.md

* little modification in test case

* style changes :)

* review changes

* remove build.sh changes

* review changes

* review changes to add more tests

* Update cpp/src/io/json/reader_impl.cu

Co-authored-by: David <[email protected]>

Co-authored-by: David <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkraus14 kkraus14 kkraus14 approved these changes

@davidwendt davidwendt davidwendt approved these changes

@vuule vuule vuule approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@cwharris cwharris cwharris approved these changes

@codereport codereport Awaiting requested review from codereport

Assignees

@rgsl888prabhu rgsl888prabhu

Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond cuIO cuIO issue libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] cudf.read_json hangs on a str data-set
8 participants
@rgsl888prabhu @GPUtester @cwharris @kkraus14 @ajschmidt8 @vuule @davidwendt @vyasr