bugGNU Octave - Bugs: bug #60138, [octave forge] (dataframe)...

 
 

bug #60138: [octave forge] (dataframe) `dataframe` incorrectly parses csv entries containing commas inside quotes

Submitter:  Tasos Papastylianou <tpapastylianou>
Submitted:  Sat 27 Feb 2021 04:21:02 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  None Assigned to:  cdemills
Originator Name:  Tasos Papastylianou Open/Closed:  * Open
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 28 Feb 2021 11:11:24 AM UTC, comment #4: 

FR, @comment #1:
The original author of csv2ell (& cell2csv and others) is Laurent Mazet. I did extend his work somewhat to cope with e.g., Excel-style ranges etc. and to keep it running in the io package.
csv2cell is a life-saver sometimes. At work with Matlab, reading large mixed-type csv files is extremely slow using importdata and textscan and often needs lots of postprocessing. csv2cell loads the same very large csv files in a jiffy and its output only needs minor polishing.

[OT]
I tend to agree with Tasos' musings about languages in general although I'm afraid Octave's niche is (very) slowly declining (or other languages simply grow faster). Too bad as especially during the last years the project has attracted several talented and active developers.
IMO a big plus of Octave (and Matlab) is that it offers a structured computing environment. E.g., for Python I found there's often a plethora of several incompatible packages for each job; not handy if at a late stage one gets stuck because at the start a less optimal choice was made.
As to R: does that support OOP? (It's used extensively at my work but I'm not very familiar with it).

Philip Nienhuis <philipnienhuis>
Group Member
Sat 27 Feb 2021 08:56:31 PM UTC, comment #3: 

Hello Tasos,
I'm glad you appreciate my efforts.
 
About the release ... that's another issue. A few years ago, Carne Draug would have helped with some of the intricacies about releasing a new version. But he resigned, and there is no more benevolent manager (that I know ) of https://octave.sourceforge.io. Look at the copyright: 2018. Latest news: 26 Aug 2018.

I had a look in the IO package. Basically, the parsing is done in C, which is much faster. Their code is about flipping some indicator when it detects a quote, so separators which are inside quote are kept into the string. It's better than mine, which is written in Octave, but it can't detect malformed strings. There is a discussion on https://stackoverflow.com/questions/18144431/regex-to-split-a-csv
about regexes to analyse a CSV string.  The subject is complex.

About R: data frames are a kind of matrix, where each column may have its own subtype, and with meta-information. Plus you have the magic of OOP to use them as first-class citizens: you ask for a matrix operation ? You get it. You ask for a list view ? You get it. You want to select by column name ? What about "myfit = lm(Y~X, data=mycsv)" ?  It's much more that syntactic sugar, it's about elegance. You focus on the relationship between two variables from a dataset, without caring for their positon.  They can be factored too. Sophisticated regressions relies on dummy variables; and R operations like 'lm' do that the right way.

Those were the objectives I wanted to reach when I started to write the data frame package. But 10 years later, I still do not have the basic implementation running as it should.

Regards

Pascal

Pascal Dupuis <cdemills>
Group Member
Sat 27 Feb 2021 07:59:05 PM UTC, comment #2: 

Hi Pascal!

First let me say that I do not mean to disparage your efforts. Clearly the dataframe package is a gargantuan effort over many years, and it's clear from the dev logs that you've invested quite a lot of time and love on it.

The reason I mention csv2cell here is not to point out that "someone else has done it correctly". It is because I imagined that a fix calling csv2cell under the hood is likely to be much less painful than trying to replicate that functionality within dataframes, hence I pointed it out hoping it might be a helpful suggestion. (and also, until this bug is patched, if any users come across this thread, it might also help them by suggesting "dataframe( csv2cell(" as a temporary workaround).

As for the other bug, as I mentioned in bug #56263, perhaps you could consider creating a release? That bug has been fixed ages ago, it's a shame that the existing release does not contain it.

Regarding the "switch to R" comment, or any other language for that matter, I don't think that's the appropriate response - and definitely not simply just to obtain a dataframe container. In fact, dataframes in R are nothing more than a list with vector elements of consistent size as its fields - meaning, in theory, one could replicate their full functionality using structs with similar vectors as fields. But of course, simply having such a construct isn't enough; one needs a nice collection of functions specialised for such a container. Which is exactly the benefit of your package (though I'm assuming your implementation does not necessarily rely on structs as mentioned here).

As someone who is well versed in octave, matlab, R, python and julia among others, I see great things in all these languages, and I most certainly do not consider octave the 'weaker player' in that list (even if the community of package contributors is smaller - and as Kai has pointed out elsewhere, I guess there are historical reasons for that, that may be worth addressing going forward, to help the community become more active and more project-independent when it comes to creating packages). Therefore I'm slightly saddened at your suggestion here (and on stackoverflow) that one should simply 'switch to R' for more serious computing...

Also, keep in mind, at least from what I see on stackoverflow, Octave has risen in popularity quite a bit in recent years -- a major driving force, I think, has been its promotion by Andrew Ng's original Machine Learning course on Coursera, which promotes Octave as the de-facto language for his course. This has been huge. But with increased exposure, there will be more users drawn to it, and therefore more users encountering bugs. This is natural, and something we, as a community, should be embracing, and nothing to shy away from, let alone admit defeat and redirect to other languages!

I'm happy to have a look and create a merge request on sourceforge if I can help. But in the meantime, please, keep up the good work, and do seek help from octave's brilliant community if you're overwhelmed for time and manpower. Don't just push them away to another language (as great as that other language might also be). Let's help make octave a tiny bit better instead, one step at a time :)

In the meantime, I think if you are happy to create a release that simply includes the existing fix for bug #56263, I know for a fact that there's at least one user of your package on stackoverflow  who's struggling with a deadline, that you'd be making very happy :)

Tasos Papastylianou <tpapastylianou>
Sat 27 Feb 2021 05:37:01 PM UTC, comment #1: 

Hello.
I'm the author, maintainer and lonely contributor of the dataframe package.

About this specific bug, what scares me is the parsing because the same element is used in the fields and between the fields. How to care with missing closing quote, and so on ?

I do not know how P. Nienhuis solved such issue in "csv2cell".

OTOH ... MatLAB was written by engineers for engineers. Meaning the availability of statistical functions was quite limited for a long time. It goes better since a few years; but if you really wish to play with big data sets, I advise you to invest your time in learning R  / RStudio which is written by statisticians. The number of available toolboxes is impressive.

Back to the subject: any help solving this issue with dataframe will be welcome. But I don't have the manpower to do it.

Regards

Pascal

Pascal Dupuis <cdemills>
Group Member
Sat 27 Feb 2021 04:21:02 PM UTC, original submission:  

This bug is in reference to this stackoverflow post: https://stackoverflow.com/q/66389166/4183191

There is another bug in that question, which is already being dealt with by #56263, namely that the second column is inappropriately truncated if it only contains strings. Apparently this was fixed in the dev repo a few years back, but still not released. This report is not for this bug.

This report is for another bug resulting from the same csv data. I reproduce the offending data below:


"TIME","GEO","UNIT","S_ADJ","NA_ITEM","Value","Flag and Footnotes"
"1995Q1","Greece","Chain linked volumes, index 2010=100","Seasonally and calendar adjusted data","Gross domestic product at market prices","72.5",""
"1995Q2","Greece","Chain linked volumes, index 2010=100","Seasonally and calendar adjusted data","Gross domestic product at market prices","73.2",""


Suppose one attempts to load this file, e.g. as


D = dataframe( 'data.csv' );


As you can see, the above should result in a field UNIT with value: "Chain linked volumes, index 2010=100"
However dataframe incorrectly treats the comma here as a delimiter, and assigns the second part of this element to the S_ADJ field instead.

As a workaround, I note that the cell2csv function from the io package deals with this correctly instead.

Therefore:


dataframe( cell2csv( 'data.csv' ) );


works as expected (barring bug #56263, that is).

Tasos Papastylianou <tpapastylianou>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by cdemills (Posted a comment)
  • -email is unavailable- added by tpapastylianou (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-02-27 mmuetzel Carbon-CopyRemoved 102357 -
    2021-02-27 mmuetzel SummaryDataframe package incorrectly parses csv entries containing commas inside quotes [octave forge] (dataframe) `dataframe` incorrectly parses csv entries containing commas inside quotes
    2021-02-27 cdemills Assigned toNone cdemills

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code