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

Option for null to "" #14

Closed
fizerkhan opened this issue Apr 23, 2013 · 25 comments
Closed

Option for null to "" #14

fizerkhan opened this issue Apr 23, 2013 · 25 comments
Labels

Comments

@fizerkhan
Copy link

The value 'null' seems to be irrelevant in CSV. It will be nicer if you can automatically convert to empty string "" in CSV file.

It could be even better and generic if it is customizable to any string or any number.

@zemirco
Copy link
Owner

zemirco commented Apr 25, 2013

What do you mean exactly? You have a null value in your JSON and for every null you want an empty String "" in the transformed CSV? Is that correct?

@fizerkhan
Copy link
Author

Yes, You are correct.

@zemirco
Copy link
Owner

zemirco commented Apr 25, 2013

I don't want to implement surprising features. People, who feed in JSON with null values expect CSV with null values.

Why don't you run a find and replace on your final CSV? Look for all null values and replace them with empty Strings "".

@fizerkhan
Copy link
Author

Yes, I can do it by either processing my input data or result(csv).
I feels, it always irrelevant to have null, undefined values in CSV file. What do you think?

@zemirco
Copy link
Owner

zemirco commented Jun 11, 2013

@fizerkhan , I won't replace null values with empty strings. I hope you understand my point of view.

@zemirco zemirco closed this as completed Jun 11, 2013
@fizerkhan
Copy link
Author

zeMirco Thanks , I understood.

@timrobertson100
Copy link

I know this is old, but rather than opening a new issue
+1 to replacing null with "".

Putting out the text "null" is both surprising and wrong in my opinion. Ideally you would be able to provide a config parameter to control what is generated for null values.

@knownasilya
Copy link
Collaborator

@timrobertson100 Would an option like transform, which would be a function that is passed the value, and you can transform and return the new value, be helpful for you for this scenario?

@timrobertson100
Copy link

It's cumbersome, but sure. Better would be just to be able to say "use '\N' for null" which is like MySQL, PostgreSQL etc. Sadly I can't get a CSV with nulls from this library into postgres using the native postgresql COPY loader because of the null handling.

@knownasilya
Copy link
Collaborator

Better would be just to be able to say "use '\N' for null" which is like MySQL, PostgreSQL etc.

That is definitely outside of the scope of this library, and much more complicated then is needed here..

@timrobertson100
Copy link

I'm sorry, but I don't understand why. It's no different to delimiter:

json2csv({ data: output.data, fields: fields, del: '|' }

Why is this so out of scope?

json2csv({ data: output.data, fields: fields, del: '|', nullVal: '\N' }

I don't mean to be awkward. It's a super nice library but doesn't produce very portable CSV files as it is.

@knownasilya
Copy link
Collaborator

Ah, ok I thought you were suggesting a DSL.. 😄

Yes, something like nullValue would be nice.

@knownasilya knownasilya reopened this Nov 23, 2015
@timrobertson100
Copy link

Thanks @knownasilya and sorry for not just putting that example at the beginning

@knownasilya
Copy link
Collaborator

So that's well and good, but it won't handle undefined values, and I don't want to implement an undefinedValue as well. So I could combine them into something like unsetValue, but then what if some one wants different values for the two.

I think the best option would be a generic solution that can be used for any situation, like transform. I could add something like json2csv({ transform: json2csv.somePresetHandler }) if we can figure out some common use cases.

@timrobertson100
Copy link

Is an undefined value one where the fields references a property that is not present? If so that is by definition a null (no value for a property) and would normally be rendered the same.

Typically with JSON libraries you have the option to render nulls as propertyName: null or exclude them completely. For example in Jackson, a common Java library, you have @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL) which controls that option.

A transform() would be nice as it would allow you to tweak your own microsyntax on a cell level, but seems like it might also bring in a can of worms (e.g. the need to pass the row number, column name etc to the function).

@knownasilya
Copy link
Collaborator

Yes it would contain the column name and value, which isn't so much a can of worms 😄.

null and undefined are not equivalent in JS (good summary on SO), since they have different meanings for what they represent, thus some might want to render them differently.

@knownasilya
Copy link
Collaborator

Just remembered about the defaultValue option. Does that not work for your use case?

Described as

String, default value to use when missing data. Defaults to ('') if not specified. (Overridden by fields[].default)

In the readme

@orzarchi
Copy link
Contributor

orzarchi commented Dec 1, 2015

defaultValue isn't working.
I would also like to output an empty string for null cells, but the following lines (160-161) prevent it:

        if (val !== undefined) {
          var stringifiedElement = JSON.stringify(val);

when val is null, the if clause still activates and JSON.stringify converts it to the string 'null'

@knownasilya
Copy link
Collaborator

@orzarchi could you submit a PR with a failing test?

@knownasilya
Copy link
Collaborator

@orzarchi that line does not prevent it, see https://github.com/zemirco/json2csv/blob/master/lib/json2csv.js#L155 (defaultValue is used with lodashGet).

Also the tests make sure that it works here: https://github.com/zemirco/json2csv/blob/master/test/index.js#L241

Also, if val was null, it would still stringify, since that is a non-coercive equality check.

@orzarchi
Copy link
Contributor

orzarchi commented Dec 5, 2015

Created PR #89 with failing test. Will try to fix it as well.

@knownasilya
Copy link
Collaborator

Ok, I merged the test, so just work off master to get it to pass 👍 Thanks!

@knownasilya
Copy link
Collaborator

@orzarchi did you have a go at the fix?

@orzarchi
Copy link
Contributor

orzarchi commented Jan 1, 2016

yup, 20 days ago #90.

@knownasilya
Copy link
Collaborator

Wow, sorry I missed it. Merged. Should have a new version out tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants