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

Set encoding when saving csv #317

Merged
merged 6 commits into from
Jun 21, 2023

Conversation

mski-iksm
Copy link
Contributor

@mski-iksm mski-iksm commented Jun 6, 2023

I've added CsvFileProcessor._encoding to allow specifying encodings when dumping in csv.

@mski-iksm mski-iksm changed the title add encoding Set encoding when saving csv Jun 6, 2023
Copy link
Collaborator

@hirosassa hirosassa left a comment

Choose a reason for hiding this comment

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

LGTM

docs/task_on_kart.rst Show resolved Hide resolved
@Hi-king
Copy link
Member

Hi-king commented Jun 7, 2023

@mski-iksm Could you consider adding some light load/save test cases?

@ukky17
Copy link
Contributor

ukky17 commented Jun 7, 2023

LGTM

docs/task_on_kart.rst Outdated Show resolved Hide resolved
docs/task_on_kart.rst Outdated Show resolved Hide resolved
mski-iksm and others added 2 commits June 10, 2023 22:08
Co-authored-by: Jumpei Ukita <ukky17@users.noreply.github.com>
@hirosassa
Copy link
Collaborator

@mski-iksm just a friendly reminder. What's this PR status?

@mski-iksm
Copy link
Contributor Author

@mski-iksm just a friendly reminder. What's this PR status?

@hirosassa I'm still working on adding tests. Please wait for few more days.

@mski-iksm
Copy link
Contributor Author

@Hi-king I've added small tests. Please re-review. Thank you.

@Hi-king
Copy link
Member

Hi-king commented Jun 21, 2023

@mski-iksm THX!LGTM!

@Hi-king Hi-king merged commit aa33e97 into m3dev:master Jun 21, 2023
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.

5 participants