Add CSV dialect support for reading and writing operations #361

Kaos599 · 2025-10-13T18:50:38Z

Introduced merge_csv_options_with_dialect function to handle merging CSV options with dialect settings.
Updated read_to_* functions to utilize the new merging logic, ensuring compatibility with user-defined CSV options.
Enhanced write functions to validate headers and merge options, providing warnings for conflicts between user options and dialect settings.
Added tests to verify correct handling of CSV dialects and option overrides in both reading and writing scenarios.

Resolves #82

- Introduced `merge_csv_options_with_dialect` function to handle merging CSV options with dialect settings. - Updated `read_to_*` functions to utilize the new merging logic, ensuring compatibility with user-defined CSV options. - Enhanced `write` functions to validate headers and merge options, providing warnings for conflicts between user options and dialect settings. - Added tests to verify correct handling of CSV dialects and option overrides in both reading and writing scenarios.

for more information, see https://pre-commit.ci

- Reformatted code in `merge_csv_options_with_dialect` and related functions for better readability by breaking long lines. - Ensured consistent formatting in test files for CSV data and dialect definitions. - Added missing newlines at the end of some files to adhere to coding standards.

for more information, see https://pre-commit.ci

dalonsoa · 2025-10-15T08:08:00Z

Many thanks for working into this! I'll review and provide feedback within the next few days.

- Corrected assertion from len(data) == 1 to len(data) == 2 - Added proper assertions for data structure validation - Test now correctly validates CSV dialect override behavior

codecov · 2025-10-17T05:31:58Z

Codecov Report

❌ Patch coverage is 96.87500% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@013c6f4). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
csvy/readers.py	94.28%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #361   +/-   ##
=======================================
  Coverage        ?   94.20%           
=======================================
  Files           ?        7           
  Lines           ?      483           
  Branches        ?        0           
=======================================
  Hits            ?      455           
  Misses          ?       28           
  Partials        ?        0

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dalonsoa

Many thanks for putting this together. It looks really good. I have a few comments to polish the implementation, but the functionality is there.

csvy/writers.py

csvy/readers.py

- Add comprehensive docstring to merge_csv_options_with_dialect with parameter and return descriptions - Reduce nesting by early return in merge_csv_options_with_dialect function - Replace hardcoded dialect_mapping with direct iteration over CSVDialectValidator.model_fields - Move merge_csv_options_with_dialect to new csvy/utils.py module - Update readers.py and writers.py to import from utils.py - Remove duplicate function definitions and unused imports - Fix Pydantic deprecation warning by accessing model_fields from class instead of instance - All pre-commit hooks pass with proper formatting and linting

- Move merge_csv_options_with_dialect to new csvy/utils.py module - Add comprehensive docstring with args/returns description - Reduce nesting by using early return pattern - Remove dialect_mapping dict, iterate directly over validator fields - Update readers.py and writers.py to import from utils - Add tests/test_utils.py with full test coverage

for more information, see https://pre-commit.ci

Kaos599 · 2025-10-19T08:07:44Z

@dalonsoa

I have tried to address all your comments from the code review!

I moved the merge_csv_options_with_dialect function to a new csvy/utils.py module with a proper docstring describing arguments and return values.
I also refactored it to reduce nesting by checking the opposite condition and returning early, and removed the unnecessary dialect mapping by getting info directly from the validator. Both readers.py and writers.py now import from utils.py.
I created tests/test_utils.py with comprehensive test cases for the utility function.

dalonsoa

Many thanks for the changes. It looks much better. However, I've found a small issue that needs to be fixed before we can merge the PR.

csvy/readers.py

- Add overrides parameter to merge_csv_options_with_dialect() - Support library-specific option names (pandas: sep, polars: separator) - Detect and resolve conflicts between user options and dialect settings - Update CSVY headers when conflicts occur with user warnings - Add comprehensive test coverage

dalonsoa

This looks really good, and I just have a couple of minor comments.

dalonsoa · 2025-10-20T09:39:45Z

csvy/writers.py

+    # Determine overrides based on data type
+    overrides = {}
+    try:
+        import pandas as pd
+
+        if isinstance(data, pd.DataFrame):
+            overrides = {"sep": "delimiter"}
+    except ImportError:
+        pass
+
+    try:
+        import polars as pl
+
+        if isinstance(data, pl.DataFrame | pl.LazyFrame):
+            overrides = {"separator": "delimiter"}
+    except ImportError:
+        pass
+
+    # For numpy arrays and lists, use standard dialect names (no overrides needed)
+


Could you separate this into a separate get_overrides function within utils.py?

dalonsoa · 2025-10-20T09:40:54Z

test_overrides.py

I guess there's something missing here? Probably it should be a test within test_utils.py rather that its own file.

dalonsoa · 2025-10-20T09:43:54Z

@all-contributors please add @Kaos599 for code, test

allcontributors · 2025-10-20T09:44:03Z

@dalonsoa

I've put up a pull request to add @Kaos599! 🎉

dalonsoa · 2025-10-20T09:44:19Z

@Kaos599 there're also some tests failing.

Kaos599 and others added 11 commits October 14, 2025 00:19

[pre-commit.ci] auto fixes from pre-commit.com hooks

f2dd626

for more information, see https://pre-commit.ci

Merge branch 'develop' of https://github.com/Kaos599/pycsvy into develop

5d796dd

fixed lint

2aeaff2

[pre-commit.ci] auto fixes from pre-commit.com hooks

44e1e0e

for more information, see https://pre-commit.ci

Fixed linting

af81b9a

Merge branch 'develop' of https://github.com/Kaos599/pycsvy into develop

2e8fdb7

[pre-commit.ci] auto fixes from pre-commit.com hooks

12e7b1e

for more information, see https://pre-commit.ci

clearing up

7a28a7e

Merge branch 'develop' of https://github.com/Kaos599/pycsvy into develop

67a8801

Kaos599 mentioned this pull request Oct 13, 2025

Use the CSV Dialect to read/write data using... csv #82

Open

dalonsoa self-requested a review October 15, 2025 08:06

Fix test assertion in test_read_csv_options_override_dialect

eeabe4e

- Corrected assertion from len(data) == 1 to len(data) == 2 - Added proper assertions for data structure validation - Test now correctly validates CSV dialect override behavior

dalonsoa requested changes Oct 17, 2025

View reviewed changes

csvy/writers.py Outdated Show resolved Hide resolved

csvy/writers.py Outdated Show resolved Hide resolved

csvy/writers.py Outdated Show resolved Hide resolved

csvy/writers.py Outdated Show resolved Hide resolved

csvy/readers.py Outdated Show resolved Hide resolved

dalonsoa added the hacktoberfest-accepted label Oct 17, 2025

Kaos599 and others added 3 commits October 17, 2025 15:53

[pre-commit.ci] auto fixes from pre-commit.com hooks

7236a26

for more information, see https://pre-commit.ci

Kaos599 and others added 4 commits October 19, 2025 13:42

ruff linting fixed

4b930fe

Merge branch 'develop' of https://github.com/Kaos599/pycsvy into develop

2fbdda4

more linting fixes

05d417d

Merge branch 'main' into develop

9b692c2

dalonsoa requested changes Oct 20, 2025

View reviewed changes

csvy/readers.py Outdated Show resolved Hide resolved

Kaos599 added 2 commits October 20, 2025 12:50

Merge branch 'develop' of https://github.com/Kaos599/pycsvy into develop

31ce2cd

Kaos599 requested a review from dalonsoa October 20, 2025 07:21

dalonsoa requested changes Oct 20, 2025

View reviewed changes

allcontributors bot mentioned this pull request Oct 20, 2025

docs: add Kaos599 as a contributor for code, and test #365

Open

Add CSV dialect support for reading and writing operations #361

Are you sure you want to change the base?

Add CSV dialect support for reading and writing operations #361

Conversation

Kaos599 commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dalonsoa commented Oct 15, 2025

Uh oh!

codecov bot commented Oct 17, 2025

Codecov Report

Uh oh!

dalonsoa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kaos599 commented Oct 19, 2025

Uh oh!

dalonsoa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dalonsoa left a comment

Choose a reason for hiding this comment

Uh oh!

dalonsoa Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

dalonsoa Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

dalonsoa commented Oct 20, 2025

Uh oh!

allcontributors bot commented Oct 20, 2025

Uh oh!

dalonsoa commented Oct 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kaos599 commented Oct 13, 2025 •

edited

Loading