-
Notifications
You must be signed in to change notification settings - Fork 72
Empty csv fix #835
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
Empty csv fix #835
Conversation
file = File.createTempFile("empty", "csv"), | ||
header = listOf("a", "b", "c"), | ||
) | ||
emptyCsvFileManualHeader shouldBe dataFrameOf("a", "b", "c").fill(0) { "" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why zeros?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, wait, it's not zeros but 0 rows of empty strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emptyCsvFileManualHeader["a"].type() shouldBe typeOf<String>()
emptyCsvFileManualHeader["a"].isEmpty() shouldBe true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this generates a DF with 3 named empty String columns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could also write something like dataFrameOf("a", "b", "c") { emptyList<String>() }
or plenty of other variants :) not sure which is the clearest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd prefer checks like above, because it takes time to imagine result of df constructor rather than see expected assertions
191f9f4
to
e277105
Compare
Please make a rebase and squash fixup before merge |
491eed7
to
d61769f
Compare
4d785f6
to
6f93a47
Compare
Generated sources will be updated after merging this PR. |
Thank you! Don't forget the milestone and other things |
they are already on the issue, as @zaleslaw recommended :) |
Fixes #834
See test for expected behavior