Skip to content

Change csv to deal in strings and convert appended values into strings - #630

Merged
wfsyre merged 6 commits into
Ubuntu-24.04from
feature-write_strings_with_csv_writer
May 1, 2026
Merged

Change csv to deal in strings and convert appended values into strings#630
wfsyre merged 6 commits into
Ubuntu-24.04from
feature-write_strings_with_csv_writer

Conversation

@wfsyre

@wfsyre wfsyre commented Mar 30, 2026

Copy link
Copy Markdown
Collaborator

Right now, the CSV writer class can only handle writing doubles to the csv table object. These get converted into strings when writing to the actual CSV. To simplify things, the CSV writer should just maintain a map of strings, which allows us to write any data type we want to the CSV as long as we convert it to a string first. It introduces some extra pre-processing we have to do to recover data from a CSV, but I think the schema of get() is already well established in scrimmage so its not a large burden. This also allows people the added control of determining how their data should be turned into a string instead of leaving it to the CSV writer to determine. I have left the scientific vs fixed schema in there but technically all that should be done before the CSV writer even sees the value. The Visitor stuff is a bit of a hacky way to leave some backward compatibility for the old Append method that only took a double.

@wfsyre
wfsyre force-pushed the feature-write_strings_with_csv_writer branch 5 times, most recently from 692998a to 88491c5 Compare April 2, 2026 23:00
@wfsyre
wfsyre force-pushed the feature-write_strings_with_csv_writer branch 2 times, most recently from 01f7748 to 307a0c1 Compare April 13, 2026 21:53

@jblee123 jblee123 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think I saw anything that wouldn't work, but there were some stylistic choices.

Comment thread src/common/CSV.cpp Outdated
Comment thread src/common/CSV.cpp Outdated
Comment thread src/common/CSV.cpp Outdated
Comment thread src/common/CSV.cpp Outdated
Comment thread src/common/CSV.cpp Outdated
Comment thread src/common/CSV.cpp Outdated
Comment thread src/common/CSV.cpp
Comment thread src/common/CSV.cpp Outdated
@wfsyre
wfsyre force-pushed the feature-write_strings_with_csv_writer branch from db71ecf to b856f10 Compare May 1, 2026 17:14
@wfsyre
wfsyre force-pushed the feature-write_strings_with_csv_writer branch from b856f10 to 5084009 Compare May 1, 2026 17:15
@wfsyre wfsyre self-assigned this May 1, 2026
@wfsyre
wfsyre merged commit 02edb3e into Ubuntu-24.04 May 1, 2026
2 checks passed
@wfsyre
wfsyre deleted the feature-write_strings_with_csv_writer branch May 1, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants