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

fix: excel data type handling #3051

Merged
merged 6 commits into from
Jun 28, 2024
Merged

fix: excel data type handling #3051

merged 6 commits into from
Jun 28, 2024

Conversation

talagluck
Copy link
Contributor

@talagluck talagluck commented Jun 24, 2024

update test .xlsx file
add excel tests to deal with columns with multiple data types

Closes #3052

@tychoish
Copy link
Collaborator

@talagluck, so I think my changes to your SLT are really just a rendering case: (e.g. strings aren't output with quotes, and sometimes floats without decimals are rendered as their integer components, and space column separators are always used.)

I'm pretty sure we get the empty string/null value handling correct, based on the code, but some more poking might be useful.

@tychoish tychoish changed the title CHORE: add tests for multiple data types in the column of an Excel table fix: excel data type handling Jun 27, 2024
@tychoish tychoish requested a review from scsmithr June 27, 2024 23:03
@talagluck
Copy link
Contributor Author

@tychoish that's all totally fine. The main thing I wanted to confirm is that the HEADING didn't turn into NULL when the column header argument was false. The tests as they are now look great.

@tychoish
Copy link
Collaborator

@scsmithr probably ready for review here!

@tychoish tychoish merged commit cd4127e into main Jun 28, 2024
26 checks passed
@tychoish tychoish deleted the tal/chore/add_excel_tests branch June 28, 2024 14:22
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.

BUG: Failed to load XLSX: Xlsx error: Parse integer error: cannot parse integer from empty string
3 participants