-
Notifications
You must be signed in to change notification settings - Fork 2
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
Replace functionality of csv with streamly #32
base: master
Are you sure you want to change the base?
Conversation
|
||
where | ||
|
||
parseLine ls = |
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.
ls
is confusing as it means lines
, should be xs
instead.
where | ||
|
||
parseLine ls = | ||
Stream.toList $ Stream.splitOn (== ',') Fold.toList $ Stream.fromList ls |
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.
This won't work with quoted fields that include escaped commas. We had frame parsing in streamly for that use case, but not sure if we have implemented it yet.
@@ -81,6 +87,28 @@ filterSanity label old new = do | |||
++ "\nOriginal groups: " ++ show old | |||
++ "\nNew groups: " ++ show new | |||
|
|||
type CSV = [[String]] | |||
|
|||
-- XXX This is ugly in performance but works for the time being. |
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 would that be the case?
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'm not truly streaming. I'm not using folds or parsers which I believe is more cleaner and the pipeline is more likely to fuse resulting in better performance.
No description provided.