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

Initial feedback after migrating to csv2 #17

Open
johnco3 opened this issue Dec 13, 2020 · 2 comments
Open

Initial feedback after migrating to csv2 #17

johnco3 opened this issue Dec 13, 2020 · 2 comments

Comments

@johnco3
Copy link

johnco3 commented Dec 13, 2020

Prior to using csv2, I was using a different aria csv parser. That parser was simple but did not support a write API. After switching over, here are the problems I have encountered so far:

I am unable to access column values by index, so taking advantate of c++ structured bindings is not possible. The following structured binding trick was possible with the previous parser and unfortunately even using iterators this is not possible with csv2:

        std::ifstream ifs(rImportFQPN);
        auto parser = std::make_unique<aria::csv::CsvParser>(ifs);
        auto bHeader = false;
        // track the cumulative distance and the lat/lons
        /*double distanceFromStart = 0.0;*/
        try {
            for (const auto& row : *parser) {
                // skip the header row
                if (!bHeader) {
                    bHeader = true;
                } else {
                    // Extract tsSecsStr(0), utc(1), callSign(2), position(3), altitude(4), speed(5), direction(6) columns
                    // C++17 structured bindings.
                    const auto&[utcSecsStr, utcStr, callSign, latLonStr,
                        altitudeStr, speedKtsStr, directionStr] =
                            std::make_tuple(row[0], row[1], row[2],
                                row[3], row[4], row[5], row[6]);

Upon further instead I have to do something along the following lines:

            // make sure we can parse the file
            if (csvReader->mmap(rImportFQPN.generic_string())) {
                const auto header = csvReader->header();
                // Extract tsSecsStr(0), utc(1), callSign(2), position(3),
                // altitude(4), speed(5), direction(6) columns
                std::string utcSecsStr, utcStr, callSign, latLonStr;
                std::string altitudeStr, speedKtsStr, directionStr;
                for (const auto& row : *csvReader) {
                    int col = 0;
                    for (const auto& cell : row) {
                        std::string value;
                        switch (col) {
                        case 0:
                            cell.read_value(utcSecsStr);
                            break;
                        case 1:
                            cell.read_value(utcStr);
                            break;
                        case 2:
                            cell.read_value(callSign);
                            break;
                        case 3:
                            // double quotes will be escaped otherwise
                            cell.read_value(latLonStr);
                            break;
                        case 4:
                            cell.read_value(altitudeStr);
                            break;
                        case 5:
                            cell.read_value(speedKtsStr);
                            break;
                        case 6:
                            cell.read_value(directionStr);
                            break;
                        default:
                            // no need to read more cols
                            break;
                        }
                        col++;
                    }

The row and cell iterators do not support operator+=(int) so unfortunately I cannot replace

                     // C++17 structured bindings.
                    const auto&[utcSecsStr, utcStr, callSign, latLonStr,
                        altitudeStr, speedKtsStr, directionStr] =
                            std::make_tuple(row[0], row[1], row[2],
                                row[3], row[4], row[5], row[6]);

with

                    const auto&[utcSecsStr, utcStr, callSign, latLonStr,
                        altitudeStr, speedKtsStr, directionStr] =
                            std::make_tuple(*row.begin(), *(row.begin()+1), *(row.begin()+2),
                                *(row.begin()+3), *(row.begin()+4), *(row.begin()+5), *(row.begin()+6));

Also there should be cbegin() and cend() const variants to each of the iterators. Visual studio returns squiggly performance warnings indicating that the modern for loop should allow the return value to bind to a const auto& row instead making a potentially large copy of the data which is returned from the csv2 API. i.e the following should be possible except the API returns a Row and not a const Row& - ditto for Column.

                for (const auto& row : *csvReader) {
                    int col = 0;
                    for (const auto& cell : row) {
@johnco3
Copy link
Author

johnco3 commented Dec 13, 2020

Also I have a row of data that completely breaksdown when it attempts to parse the position that contains a comma that needs to be skipped. I specified the quote character needs to be escaped.
the column in question is "32.97908,-79.889282" - including the pair of quotation marks that should be part of the column. When this is restored I get the value ""\32.97908" and "-79.889282"" - making my subsequent columns incorrect

Timestamp,UTC,Callsign,Position,Altitude,Speed,Direction
. . .
1545497433,2018-12-22T16:50:33Z,ETD9011,"32.97908,-79.889282",4850,281,76

@johnco3
Copy link
Author

johnco3 commented Dec 13, 2020

Another issue has to do with std::min and std::max on windows. In my source cpp file that includes csv2.hpp needs to have a preprocessor define NOMINMAX in order to work around this as csv2 includes windows.h. See this for details.

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

No branches or pull requests

1 participant