Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

add dictionary serialization for csv-writer #515

Merged
merged 2 commits into from
Oct 10, 2021

Conversation

ritchie46
Copy link
Collaborator

@ritchie46 ritchie46 commented Oct 9, 2021

This PR adds csv serializations for dictionary encoded arrays. I've only implemented them for typically rust indexes usize atm. It would be trivial to add other indexes, but if no-one (don't know how they are used in the wild) uses them it might not be worth the compilation.

closes #508

@ritchie46 ritchie46 added the enhancement An improvement to an existing feature label Oct 9, 2021
@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #515 (a63fbe4) into main (e77d4e1) will decrease coverage by 0.00%.
The diff coverage is 75.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #515      +/-   ##
==========================================
- Coverage   80.18%   80.17%   -0.01%     
==========================================
  Files         381      381              
  Lines       23665    23693      +28     
==========================================
+ Hits        18975    18997      +22     
- Misses       4690     4696       +6     
Impacted Files Coverage Δ
src/io/csv/write/serialize.rs 52.58% <70.83%> (+5.27%) ⬆️
tests/it/io/csv/write.rs 96.22% <100.00%> (+0.39%) ⬆️
src/compute/arithmetics/time.rs 44.89% <0.00%> (-2.05%) ⬇️
src/bitmap/utils/slice_iterator.rs 92.53% <0.00%> (-1.50%) ⬇️
src/io/ipc/write/common.rs 75.64% <0.00%> (-0.65%) ⬇️
src/io/avro/read/schema.rs 55.95% <0.00%> (+1.19%) ⬆️
src/types/index.rs 78.78% <0.00%> (+6.06%) ⬆️

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 e77d4e1...a63fbe4. Read the comment docs.

Copy link
Owner

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

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

Thanks a lot, Ritchie!

Almost there imo:

  1. need to add the bound checks
  2. some tests :P

src/io/csv/write/serialize.rs Outdated Show resolved Hide resolved
@jorgecarleitao jorgecarleitao merged commit 25ddb0d into jorgecarleitao:main Oct 10, 2021
@jorgecarleitao jorgecarleitao added feature A new feature and removed enhancement An improvement to an existing feature labels Oct 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Dictionary to csv writer
2 participants