Duplicate header names cause parsing to overwrite data

[jannah]

I am testing a system I am building using PapaParse. One scenario I am looking it is if more than one column shared the same header. What I am seeing is that only the last column would be included in the result.
http://jannah.github.io/MetaReader
Is there a way to rename the duplicate columns?

[mholt]

Cool project!

I've been aware of this issue way back since Papa Parse 1, and my recommendation ever since then is to just turn off the header row feature. It has been satisfactory for most users. (The header row feature is more of a convenience for well-structured, predictable data anyway.)

With header row off, it's just an array of arrays so duplicate field names are no problem.

[jannah]

What if a counter was appended to duplicate names? that should be easy to do I think.
Getting the data back in clean json is very useful. It saves the hassle of converting things to json again.

[mholt]

Alright, that's a fair solution I think -- the field names are returned in order in the meta object anyway, so the user will always be able to know which fields exist and what order they were in. So adding a counter may not be too breaking.

How would it work? If we have two fields "Foo", do we call one "Foo1" and the other "Foo2", or do we just have "Foo2"? (In other words, do we add a counter to the first field as well?) And what if there is already a field called "Foo2"? The logic does get slightly sticky... a nested loop, for sure...

These fields:

Foo,Foo,Foo2,Foo2,Foo3

Would become:

Foo,Foo2,Foo22,Foo222,Foo3

Or:

Foo1,Foo21,Foo22,Foo23,Foo3

Doable, but icky, no?

I hesitate to just go forward with this feature because of the complexities, unless there's a simpler way to handle it...

[jannah]

I played around with it. check it out
http://jsfiddle.net/jannah/d67cfssk/4/

My principles:

• first occurrence remains untouched
• append separator + counter (e.g. _1) to duplicates
• check if a header with the newly proposed name already exists. if it does, duplicate the separator. Loop through this until the new header is new. (e.g. text___1)

[mholt]

Phew, finally getting around to issues again. Sorry for the wait.

I took a look at your fiddle. I like how it keeps the field name fairly clean, even when it encounters duplicate field names. However, something feels strange about modifying the field name in the middle rather than appending to the end (which also feels clunky, as I mentioned above). Maybe it's that I don't know which field is the original when I look at "Test_1" and "Test__1" -- I could know by reading documentation, but most users won't know why their field got renamed; it's extra cognitive overhead for the developer, too, when they have to plan for their fields being renamed on-the-fly. So even though your algorithm is a good one, the user experience angel sitting on my shoulder is shaking his head at me...

Do you think we could save ourselves all this pain by just disabling the header row when we can't be sure the input won't have duplicate column names?

[jannah]

I agree with you that there is a potential cognitive overload in the case you highlighted. I put it there as as a potential case, albeit very rare (probably). I left the separator as a parameter so it can be changed easily (e.g. copy1 instead of _ like ipython notebooks).
I think an easier solution is to to change the input manually before loading. In the mean time, I think it's worth pointing out in the documentation that headers need to be unique when dealing with headers. What do you think?

[mholt]

I think that is an excellent solution -- it puts more control in the hands of the developer and it isn't confusing. And you are right, I should have mentioned this in the docs. I'll leave the issue open until I do that. Thanks for your feedback!

[Overdrivr]

That's not so rare, happened to me yesterday ;)
For data pipelines, this is very annoying because it silently fails (in my case, the column in question was an "id" one, breaking all relationships in my data model).
At the bare minimum, would it be acceptable to have an error returned because fundamentally the behavior is undefined ?

[fortydegrees]

Likewise had this issue. I crudely applied the fix in @jannah 's fiddle to the parse function in papaparse.js

Line 1460, after:

if (!input)
    return returnable();

Updated to append "_" to duplicate fields:

// Rename headers if there are duplicates
let firstLine = input.split(newline)[0]
let separator = '_';
let headers = firstLine.split(delim) //headers array
let header_map = []; //map of headers after resolving duplicates
let header_count = {}; //count the headers
//process headers
for (var i in headers) {
    var header = headers[i]
    //check if header already exists
    if (typeof header_count[header] === 'undefined') {
        //print('new column ' + header);
        header_count[header] = 1;
    } else {
        //rename header if existed starting with _1
        var current_count = header_count[header];
        header_count[header] += 1;
        //append counter to header
        let temp_header = header + separator;
        //in case the porposed name already exists in the original csv
        while (headers.indexOf(temp_header + current_count) > -1) {
            temp_header += separator
            //print(temp_header)
        }
        header = temp_header + current_count;
        header_count[header] = 1;
    }
    //add new header to header_map
    header_map.push(header)
}

I don't see the downside to adding an allowDuplicates config flag to enable this behaviour in the standard library.

[pokoli]

Could you upload a PR with a test case that reproduces the error and a correction for it? Thanks

[fortydegrees]

I've submitted a pull request with a test case & correction: #956

I tried to add a renameDuplicateHeaders config option but left it out as I am doing the processing in the Parse.parse function where I don't have the ability to read the config. I'm not sure it fits the styling of the codebase to modify the input outside of that function so have left it out.

There was a bug in my original comment with the processing happening at the wrong time, but have since edited it so that the bug no longer occurs.