Emit an error if row and header size mismatch #622
Conversation
satabin
force-pushed
the
fix/emit-row-header-size-mismatch
branch
from
cb7f54e
to
812c16d
Compare
| @@ -217,6 +221,27 @@ package object csv { | |||
| } | |||
| } | |||
|
|
|||
| /** Encode a specified type into a CSV prepending the given headers. */ | |||
| def encodeWithGivenHeaders[T]: PartiallyAppliedEncodeWithGivenHeaders[T] = | |||
There was a problem hiding this comment.
Wdyt about renaming to encodeWithHeaders? We already have encodeWithoutHeaders, so the naming feels a bit more consistent then the WithGiven.
No strong opinion on this though, just noticed it, decide as you see fit.
There was a problem hiding this comment.
I wanted to keep consistent naming between the high and low level naming. Since the low level naming cannot be writeWithHeaders(this is the deprecated one) and high-level cannot be encodeGivenHeaders (this is the deprecated one), I settled for merging both.
| val input = List( | ||
| Row(NonEmptyList.of("a", "b", "c"), Some(1)), | ||
| Row(NonEmptyList.of("d", "e"), Some(2)), | ||
| Row(NonEmptyList.of("f", "g", "h", "i"), Some(3)), | ||
| Row(NonEmptyList.of("j", "k", "l"), Some(4)) | ||
| ) | ||
| val headers = NonEmptyList.of("first", "second", "third") |
There was a problem hiding this comment.
| val input = List( | |
| Row(NonEmptyList.of("a", "b", "c"), Some(1)), | |
| Row(NonEmptyList.of("d", "e"), Some(2)), | |
| Row(NonEmptyList.of("f", "g", "h", "i"), Some(3)), | |
| Row(NonEmptyList.of("j", "k", "l"), Some(4)) | |
| ) | |
| val headers = NonEmptyList.of("first", "second", "third") | |
| val headers = NonEmptyList.of("first", "second", "third") | |
| val input = List( | |
| Row(NonEmptyList.of("a", "b", "c"), Some(1)), | |
| Row(NonEmptyList.of("d", "e"), Some(2)), | |
| Row(NonEmptyList.of("f", "g", "h", "i"), Some(3)), | |
| Row(NonEmptyList.of("j", "k", "l"), Some(4)) | |
| ) |
I find this test case slightly annoying to read, but except for this reordering (so it matches output order) and maybe adding some unapplySeq in cats itself, I found no better.
There was a problem hiding this comment.
One more thought on this snippet (not to be done in this PR though): Maybe a row"…, …, …" string interpolator could be nice, especially for our tests?
There was a problem hiding this comment.
Tried adding unapplySeq to cats, unfortunately it doesn't work because NonEmptyList is a case class and hence has a generated unapply that takes precedence. Would need a dedicated extractor object.
Fixes #621
A new method is created and the old one is deprecated to keep binary compatibility.