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

Simplify write_csv by removing unnecessary writer/impl classes #9089

Merged
merged 34 commits into from
Nov 18, 2021
Merged

Simplify write_csv by removing unnecessary writer/impl classes #9089

merged 34 commits into from
Nov 18, 2021

Conversation

cwharris
Copy link
Contributor

@cwharris cwharris commented Aug 21, 2021 •
edited
Loading

Depends on #9040 and (unfortunately) #9041

@cwharris
simplify io/functions.cpp data source/sink factories
9c72e56
@cwharris
begin replacing csv_reader with pure functions
9e92ca2
@cwharris
pass parse_options explicitly in csv_reader
6492349
@cwharris
replace csv reader impl::select_data_types with pure function
3e365b5
@cwharris
replace csv reader impl::column_flags_ member with local variable
a4497c0
@cwharris
make csv reader impl::find_first_row_start a standalone function
6d708b7
@cwharris
make csv reader impl:col_names_ a local variable
26e37e2
@cwharris
replace csv reader impl::num_records with local variable.
9d84753
@cwharris
convert csv reader impl ::num_actual_columns and ::num_active_columns…
7ce862e 
… to local variables
@cwharris
remove csv reader class and impl class in favor of fucntions
9010fe1
@cwharris
rearrange some functions to delete some unneccessary declarations.
7cda106
@cwharris
Merge branch 'branch-21.10' of github.com:rapidsai/cudf into io-funct…
884bde6 
…ions-simplify
@cwharris
remove filepath-related logic from csv and json readers
88e2399
@cwharris
remove filepath logic from avro, parquet, orc readers
62b9520
@cwharris
move range size padding calculation out of json/csv reader and in to …
fb01294 
…json/csv options
@cwharris
remove filepaths from json reader
d422aeb
@cwharris
Merge branch 'io-functions-simplify' into io-simplify-csv
a67150e
@cwharris
remove unncessary csv writer class
4808a43
@cwharris
replace csv::writer_impl out_sink_ member with local variable
9688d77
@cwharris
replace csv::writer_impl members with local variables
7840dc5
@cwharris
remove unnecessary csv writer impl class
16e4eee
@cwharris
replace writer_impl.hpp with more appropriately named durations.hpp
9994ea3
@github-actions github-actions bot added Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Aug 21, 2021
@codecov
Copy link

codecov bot commented Aug 21, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.02@31f92d7). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-22.02    #9089   +/-   ##
===============================================
  Coverage                ?   10.49%           
===============================================
  Files                   ?      119           
  Lines                   ?    20297           
  Branches                ?        0           
===============================================
  Hits                    ?     2130           
  Misses                  ?    18167           
  Partials                ?        0

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 31f92d7...4fa0f44. Read the comment docs.

@github-actions github-actions bot removed the Python Affects Python cuDF API. label Aug 24, 2021
@vuule vuule mentioned this pull request Sep 28, 2021
Closed
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@cwharris cwharris changed the base branch from branch-21.10 to branch-21.12 October 26, 2021 15:19
@cwharris cwharris marked this pull request as ready for review November 15, 2021 20:14
@cwharris cwharris requested a review from a team as a code owner November 15, 2021 20:14
@cwharris cwharris changed the base branch from branch-21.12 to branch-22.02 November 15, 2021 20:16
robertmaynard
robertmaynard requested changes Nov 15, 2021
View reviewed changes
cpp/src/io/csv/durations.hpp Show resolved Hide resolved
cpp/src/io/csv/writer_impl.cu Show resolved Hide resolved
cpp/src/io/csv/writer_impl.cu Outdated Show resolved Hide resolved
cpp/src/io/csv/writer_impl.cu Outdated Show resolved Hide resolved
@vuule
Copy link
Contributor

vuule commented Nov 18, 2021

rerun tests

@vuule
Copy link
Contributor

vuule commented Nov 18, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit d4ff518 into rapidsai:branch-22.02 Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vuule vuule vuule approved these changes

@robertmaynard robertmaynard robertmaynard approved these changes

@codereport codereport Awaiting requested review from codereport codereport is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
Labels
cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cwharris @vuule @robertmaynard