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

'columns' unparse config not working when first object doesn't have all properties #918

Closed
TheSlimvReal opened this issue Feb 24, 2022 · 6 comments · Fixed by #919
Closed

Comments

@TheSlimvReal
Copy link
Contributor

TheSlimvReal commented Feb 24, 2022

When using the columns unparse config option together with a data array where the first object does not have all properties that are listed in the columns array, then not all columns of the columns array are used even if later objects have these values.

E.g.

const columns= ["First", "Second", "Third"];
const rows= [
  { First: "1-1"},
  {
    First: "2-1",
    Second: "2-2",
    "Third": "2-3",
  },
];
const result = Papa.unparse(
  { data: rows},
  {
    header: true,
    columns: columns,
  }
);
console.log(result)

The result is

First
1-1
2-1

instead of the expected

First,Second,Third
1-1,,
2-1,2-2,2-3
@pokoli
Copy link
Collaborator

pokoli commented Feb 24, 2022

I guess the problem is that the columns array is modified on the first colum when it should not be.
Could you please upload a PR to fix it which includes a minimum test case (your example sounds like a good solution)

@TheSlimvReal
Copy link
Contributor Author

TheSlimvReal commented Feb 24, 2022

Thanks for the quick response. I will try to make the PR. As this would be my first contribution to Papaparse, do you @pokoli have some pointers for me where I would have to look for a solution/implement the tests and the code?

@pokoli
Copy link
Collaborator

pokoli commented Feb 24, 2022

@TheSlimvReal That will be great.
THinks to note:

  • You do not need to update the minified file as this will be done by the release process.
  • There are already tests for unparse, make sure to add yours at the end (feel free to use them as reference).
  • Feel free to ask any other doubts you may have ;-)

@TheSlimvReal
Copy link
Contributor Author

TheSlimvReal commented Feb 24, 2022

After going through the tests cases I found this one:

{
  description: "Column option used to manually specify keys",
  notes: "...",
  input: [{a: 1, b: '2'}, {}, {a: 3, d: 'd', c: 4,}],
  config: {columns: ['a', 'b', 'c']},
  expected: 'a,b,c\r\n1,2,\r\n\r\n3,,4'
},

Which made me realize that with

...
const result = Papa.unparse(
  rows,   // <-- pass the data object directly instead of { data: rows }
  {
    header: true,
    columns: columns,
  }
);
...

My above example works.

Therefore my question @pokoli: Is it a bug that it does not work when passing an object instead of an array or is this behavior intended?

@TheSlimvReal
Copy link
Contributor Author

anyway I created a PR that resolves the issue #919

@TheSlimvReal
Copy link
Contributor Author

Generally, I am wondering whether the options columns and fields are not the same thing and columns could therefore be removed?

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

Successfully merging a pull request may close this issue.

2 participants