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

test: Add csv reading test for duckdb_read_csv(na.strings = ) #10

Merged
merged 4 commits into from
Feb 24, 2024

Conversation

Tmonster
Copy link
Contributor

prompted by this issue
duckdb/duckdb#8590

Copy link
Collaborator

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks. It's not immediately obvious what this test tests. Would it help writing a literal CSV string to a file, or pointing to a file that is already part of the codebase?

tests/testthat/test_read.R Outdated Show resolved Hide resolved
@krlmlr krlmlr force-pushed the new_csv_test_na_strings branch from c892158 to bf309a8 Compare November 8, 2023 13:01
@Tmonster
Copy link
Contributor Author

Tmonster commented Nov 8, 2023

It just tests to make sure null values are properly read from a csv file using the parameter passed when na_strings is set

Potentially some users write to CSV using R and read using duckdb, so it's nice to make sure NA values are respected when the na_strings parameter is passed.

@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.89%. Comparing base (10b3416) to head (1bc88c6).

❗ Current head 1bc88c6 differs from pull request most recent head de3dd41. Consider uploading reports for the commit de3dd41 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #10   +/-   ##
=======================================
  Coverage   85.89%   85.89%           
=======================================
  Files         106      106           
  Lines        3602     3602           
=======================================
  Hits         3094     3094           
  Misses        508      508           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krlmlr krlmlr force-pushed the new_csv_test_na_strings branch from a0c7ed6 to 660fc86 Compare February 22, 2024 11:30
@krlmlr krlmlr force-pushed the new_csv_test_na_strings branch from 660fc86 to de3dd41 Compare February 23, 2024 17:00
@krlmlr krlmlr changed the title add csv reading test for na.strings test: Add csv reading test for duckdb_read_csv(na.strings = ) Feb 24, 2024
@krlmlr krlmlr merged commit 4a82721 into duckdb:main Feb 24, 2024
13 checks passed
@krlmlr
Copy link
Collaborator

krlmlr commented Feb 24, 2024

Thanks!

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.

3 participants