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

Re-enable JSON tests #8843

Merged
merged 2 commits into from
Jul 27, 2021
Merged

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Jul 23, 2021

#8403 disabled a large portion of JSON tests.
This PR reverts the accidental change in that PR.

@vuule vuule added bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. cuIO cuIO issue non-breaking Non-breaking change labels Jul 23, 2021
@vuule vuule self-assigned this Jul 23, 2021
@vuule vuule requested a review from jdye64 July 24, 2021 05:24
@vuule vuule marked this pull request as ready for review July 24, 2021 05:24
@vuule vuule requested a review from a team as a code owner July 24, 2021 05:24
@vuule vuule requested review from harrism and hyperbolic2346 July 24, 2021 05:24
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #8843 (6f441f0) into branch-21.10 (18f7c01) will decrease coverage by 0.09%.
The diff coverage is 0.00%.

❗ Current head 6f441f0 differs from pull request most recent head 6b37786. Consider uploading reports for the commit 6b37786 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.10    #8843      +/-   ##
================================================
- Coverage         10.67%   10.58%   -0.10%     
================================================
  Files               110      116       +6     
  Lines             18271    18650     +379     
================================================
+ Hits               1951     1974      +23     
- Misses            16320    16676     +356     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/lists.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/numerical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/struct.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/dataframe.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
... and 24 more

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 32f46f0...6b37786. Read the comment docs.

Copy link
Contributor

@jdye64 jdye64 left a comment

Choose a reason for hiding this comment

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

This was my fault. I had those tests commented out so I could run GDB on a single other test and looks like I forgot to uncomment them when I was done.

@vuule
Copy link
Contributor Author

vuule commented Jul 26, 2021

rerun tests

@harrism
Copy link
Member

harrism commented Jul 27, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c0a7f58 into rapidsai:branch-21.10 Jul 27, 2021
@vuule vuule deleted the bug--disabled-json-tests branch July 27, 2021 01:39
rapids-bot bot pushed a commit that referenced this pull request Aug 4, 2021
… of `data_type` objects (#8856)

Goal of the PR is to enable CSV to read columns as decimal, and to replace the string-based `dtype` part of the API. `data_type` based API is needed because we need to specify scale for decimal columns, and doing this via a string that describes the type is 💩 

Changes in the PR:
- Added overloads to `dtype` related getters/setters to also take a vector or a map of `data_type` objects.
In case of CSV, vector of `data_type`s was already supported. Reworked the implementation to support different use cases that the "dtype-as-string" code path supports.
- Fixed naming of compression option setter.
- Added `parse_dates` option to make up for the special strings that CSV supported to denote that a column needs to be parsed as hexadecimal (the option to pass strings is to be removed).
- Changed naming of `infer_date` option to `parse_dates`.
- Updated all CSV and JSON tests to use the new APIs.

Breaking because API to specify date columns has been renamed to match the new `parse_hex` API; renamed from `infer_date` to `parse_dates`

Depends on #8843

Authors:
  - Vukasin Milovanovic (https://github.com/vuule)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Ram (Ramakrishna Prabhu) (https://github.com/rgsl888prabhu)
  - AJ Schmidt (https://github.com/ajschmidt8)
  - Elias Stehle (https://github.com/elstehle)

URL: #8856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working cuIO cuIO issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants