October 28, 2011 Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Formal review of Jesse Phillips's CSV parser module begins today and runs through November . Following that, a vote will take place for one week. Please post any comments about this library in this thread. Docs: http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html Code: https://github.com/he-the-great/phobos/tree/csv |
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to dsimcha | On Fri, 28 Oct 2011 16:18:27 +0300, dsimcha <dsimcha@yahoo.com> wrote: > Docs: > http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html Documentation should have examples of basic usage at the top. It should be possible to figure out how to use a CSV parser within 10 seconds of looking at the docs. It looks like this is only a CSV parser, but not a CSV formatter? That's pretty weird. It'd be like std.xml or std.json only having reading functionality. -- Best regards, Vladimir mailto:vladimir@thecybershadow.net |
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to dsimcha | dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv
Could csvText!Struct(...) be generalized to also support Tuples and (eventually) classes?
|
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to dsimcha | On 10/28/2011 9:18 AM, dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv
I'll kick this off with my review of the documentation (I'll review the implementation later):
"Header may be provided as first line in file" -> "A header may be provided as first line in file"?
csvText: Shouldn't heading be a generic range of strings instead of a string[]?
Why should the order of the heading provided to csvText matter? csvText should simply rearrange its results.
In csvNextToken: "The expected use of this would be to create a parser. And may also be useful when handling errors within a CSV file." Please re-word this sentence so it's grammatically correct.
|
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to Piotr Szturmaj | On 2011-10-28 15:25, Piotr Szturmaj wrote: > dsimcha wrote: >> Formal review of Jesse Phillips's CSV parser module begins today and >> runs through November . Following that, a vote will take place for one >> week. Please post any comments about this library in this thread. >> >> Docs: >> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html >> >> Code: >> https://github.com/he-the-great/phobos/tree/csv > > Could csvText!Struct(...) be generalized to also support Tuples and > (eventually) classes? Sounds more like a serialization library with a CSV archive type to me. -- /Jacob Carlborg |
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to dsimcha | On 2011-10-28 15:18, dsimcha wrote: > Formal review of Jesse Phillips's CSV parser module begins today and > runs through November . Following that, a vote will take place for one > week. Please post any comments about this library in this thread. > > Docs: > http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html > > Code: > https://github.com/he-the-great/phobos/tree/csv If a header is specified, is it possible to iterate over the content as an associative array, something like this: string str = "a,b,c\nHello,65,63.63\nWorld,123,3673.562"; auto records = csvText(str, ["b"]); foreach (header, value ; records) {} Or accessing a value by header: foreach (record ; records) { auto value = record["a"]; auto val = record.b; // perhaps using opDispatch as well } -- /Jacob Carlborg |
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jacob Carlborg | Jacob Carlborg wrote: > On 2011-10-28 15:25, Piotr Szturmaj wrote: >> dsimcha wrote: >>> Formal review of Jesse Phillips's CSV parser module begins today and >>> runs through November . Following that, a vote will take place for one >>> week. Please post any comments about this library in this thread. >>> >>> Docs: >>> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html >>> >>> Code: >>> https://github.com/he-the-great/phobos/tree/csv >> >> Could csvText!Struct(...) be generalized to also support Tuples and >> (eventually) classes? > > Sounds more like a serialization library with a CSV archive type to me. > I suppose you misunderstood my intentions. Here's an example from the docs: string str = "Hello,65,63.63\nWorld,123,3673.562"; struct Layout { string name; int value; double other; } auto records = csvText!Layout(str); foreach(record; records) { writeln(record.name); writeln(record.value); writeln(record.other); } I want to occasionally write: alias Tuple!(string, int, double) Layout; // instead of struct It should be easy to extend, but it would be nice if we have had general construct in Phobos. So instead of is(Contents == struct) @ https://github.com/he-the-great/phobos/blob/csv/std/csv.d#L433 there should be isComposite!Contents and FieldTypeTuple should be generalized (if it isn't) to structs, classes and tuples. I don't know if Tuple as a struct doesn't return valid fields from FieldTypeTuple now. |
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to Piotr Szturmaj | Le 28/10/2011 16:09, Piotr Szturmaj a écrit :
> Jacob Carlborg wrote:
>> On 2011-10-28 15:25, Piotr Szturmaj wrote:
>>> dsimcha wrote:
>>>> Formal review of Jesse Phillips's CSV parser module begins today and
>>>> runs through November . Following that, a vote will take place for one
>>>> week. Please post any comments about this library in this thread.
>>>>
>>>> Docs:
>>>> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>>>>
>>>> Code:
>>>> https://github.com/he-the-great/phobos/tree/csv
>>>
>>> Could csvText!Struct(...) be generalized to also support Tuples and
>>> (eventually) classes?
>>
>> Sounds more like a serialization library with a CSV archive type to me.
>>
>
> I suppose you misunderstood my intentions. Here's an example from the docs:
>
> string str = "Hello,65,63.63\nWorld,123,3673.562";
> struct Layout {
> string name;
> int value;
> double other;
> }
>
> auto records = csvText!Layout(str);
>
> foreach(record; records) {
> writeln(record.name);
> writeln(record.value);
> writeln(record.other);
> }
>
> I want to occasionally write:
>
> alias Tuple!(string, int, double) Layout; // instead of struct
>
> It should be easy to extend, but it would be nice if we have had general
> construct in Phobos. So instead of is(Contents == struct) @
> https://github.com/he-the-great/phobos/blob/csv/std/csv.d#L433 there
> should be isComposite!Contents and FieldTypeTuple should be generalized
> (if it isn't) to structs, classes and tuples. I don't know if Tuple as a
> struct doesn't return valid fields from FieldTypeTuple now.
That would be great !
|
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to dsimcha | On 10/28/11 10:18 AM, dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv
I like the idea of reading a CSV into a struct, or treating all fields as ints.
But, in my experience, CSVs are never perfect... because humans fill them, and humans aren't perfect or don't know "Oh, of course I must fill only numbers in this column, otherwise programs will explode".
So although it's a nice functionality, I think just returning everything as strings is more than enough.
|
October 28, 2011 Re: Review of Jesse Phillips's CSV Parser | ||||
---|---|---|---|---|
| ||||
Posted in reply to dsimcha | dsimcha Wrote: > I'll kick this off with my review of the documentation (I'll review the implementation later): > > "Header may be provided as first line in file" -> "A header may be provided as first line in file"? > > csvText: Shouldn't heading be a generic range of strings instead of a string[]? I should revisit that. It will still need to be random access and possible some others, but doesn't need to be an array. > Why should the order of the heading provided to csvText matter? csvText should simply rearrange its results. > > In csvNextToken: "The expected use of this would be to create a parser. And may also be useful when handling errors within a CSV file." Please re-word this sentence so it's grammatically correct. |
Copyright © 1999-2021 by the D Language Foundation