Skip to content

Support CsvMerge to specify dtype parameter.(Fixes #647)#659

Merged
skxeve merged 2 commits intomasterfrom
feature/647-bugfix-csvmerge-dtype
Apr 1, 2026
Merged

Support CsvMerge to specify dtype parameter.(Fixes #647)#659
skxeve merged 2 commits intomasterfrom
feature/647-bugfix-csvmerge-dtype

Conversation

@skxeve
Copy link
Copy Markdown
Collaborator

@skxeve skxeve commented Mar 25, 2026

Brief

Fixes #647

Points to Check

  • Any unexpected impact
  • The overall quality is trending positively

Test

Confirmed

Pass ALL CI.

Review Limit

  • Write review limit on pull request title.

@skxeve skxeve self-assigned this Mar 25, 2026
@skxeve skxeve added the enhancement Improvement to an existing feature label Mar 25, 2026
@skxeve skxeve requested a review from Tsuyoposon March 31, 2026 06:44
@x-chi
Copy link
Copy Markdown
Collaborator

x-chi commented Mar 31, 2026

[should]
You need to add an explanation of the new dtype parameter to the documentation, such as docs/modules/csv_merge.md.

[memo]
Invalid input to dtype: The Arguments class defines dtype: str | dict, but if a string or dictionary in an unacceptable format is passed to Pandas/Dask's read_csv, an exception will be raised at runtime.

@skxeve
Copy link
Copy Markdown
Collaborator Author

skxeve commented Apr 1, 2026

@x-chi
Thanks for the review!
I have updated docs/modules/csv_merge.md to include an explanation of the dtype parameter.

Regarding the exception when an invalid dtype is passed:
Since cliboa is an ETL framework, users typically identify such configuration errors during the initial testing phase of their scenarios. Therefore, I believe this behavior is acceptable.

Furthermore, strictly validating the dtype format to match the evolving specifications of Pandas or Dask would involve high maintenance costs. I think it is better to keep the constraints loose on the cliboa side and let the underlying libraries handle the specific validation.

Thank you for the feedback!

Copy link
Copy Markdown
Collaborator

@x-chi x-chi left a comment

Choose a reason for hiding this comment

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

Thank you for implementing it. I was also able to confirm the manual.

@skxeve skxeve merged commit 6c8b98d into master Apr 1, 2026
6 checks passed
@skxeve skxeve deleted the feature/647-bugfix-csvmerge-dtype branch April 1, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Add dtype support for CsvMerge

3 participants