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

Allow specifying True/False values for boolean fields #415

Closed
akariv opened this Issue May 2, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@akariv
Contributor

akariv commented May 2, 2017

Right now the list of string terms indicating a true or false value are hard-coded in the spec, and culturally biased towards English (even the 'case insensitive' note, which could make very little sense for other languages).
In order to better support a multilingual environment, it would be better to allow the user to provide the list of true & false values.
Also, I would recommend removing the lowercasing preprocessing and explicitly list ["YES", "Yes", "yes"] instead of just "yes".

@akariv akariv added the Table Schema label May 2, 2017

@akariv akariv changed the title from Allow specifying True/False values for boolean values to Allow specifying True/False values for boolean fields May 2, 2017

@rufuspollock

This comment has been minimized.

Show comment
Hide comment
@rufuspollock

rufuspollock May 24, 2017

Contributor

@akariv good point. Do you have a suggestion for exact wording of the updated spec?

Contributor

rufuspollock commented May 24, 2017

@akariv good point. Do you have a suggestion for exact wording of the updated spec?

@pwalsh

This comment has been minimized.

Show comment
Hide comment
@pwalsh

pwalsh May 29, 2017

Member

@akariv I agree on:

  • not processing case (the array of values is explicit)
  • allowing custom arrays of boolean values, in the same manner as missingValues.

Perhaps:

  • trueValues: [ "true", "True", "TRUE" ]
  • falseValues: [ "false, "False", "FALSE" ]

As the defaults. And user can set:

{
 ... a field ...,
 "trueValues": [ "y" ],
 "falseValues": [ "n" ]
}
Member

pwalsh commented May 29, 2017

@akariv I agree on:

  • not processing case (the array of values is explicit)
  • allowing custom arrays of boolean values, in the same manner as missingValues.

Perhaps:

  • trueValues: [ "true", "True", "TRUE" ]
  • falseValues: [ "false, "False", "FALSE" ]

As the defaults. And user can set:

{
 ... a field ...,
 "trueValues": [ "y" ],
 "falseValues": [ "n" ]
}

@pwalsh pwalsh self-assigned this May 29, 2017

@pwalsh pwalsh added this to the v1.0 milestone May 29, 2017

@akariv

This comment has been minimized.

Show comment
Hide comment
@akariv

akariv May 29, 2017

Contributor

Looks good @pwalsh

Contributor

akariv commented May 29, 2017

Looks good @pwalsh

@pwalsh

This comment has been minimized.

Show comment
Hide comment
@pwalsh

pwalsh May 29, 2017

Member

@roll @akariv should this be at the table level, like missingValues, or the field level? (I'd go for table level)

Member

pwalsh commented May 29, 2017

@roll @akariv should this be at the table level, like missingValues, or the field level? (I'd go for table level)

@pwalsh

This comment has been minimized.

Show comment
Hide comment
@pwalsh

pwalsh Jun 19, 2017

Member

@rufuspollock assigning back to you, as I think this is ready. trueValues and falseValues should be applicable anywhere that missingValues is. I can do the actual PR if you are signing off on this.

Member

pwalsh commented Jun 19, 2017

@rufuspollock assigning back to you, as I think this is ready. trueValues and falseValues should be applicable anywhere that missingValues is. I can do the actual PR if you are signing off on this.

@pwalsh pwalsh self-assigned this Jun 19, 2017

@rufuspollock

This comment has been minimized.

Show comment
Hide comment
@rufuspollock

rufuspollock Jun 21, 2017

Contributor

@pwalsh great suggestions and please go ahead with a PR.

My only hesitation here is whether this should be pattern vs core spec - it is yet another thing for implementors to do and one option would be to get a sense of importance from a pattern before we push into the spec.

I'm happy to be swayed either way on this one (i.e. spec vs pattern and candidate for v1.1)

Contributor

rufuspollock commented Jun 21, 2017

@pwalsh great suggestions and please go ahead with a PR.

My only hesitation here is whether this should be pattern vs core spec - it is yet another thing for implementors to do and one option would be to get a sense of importance from a pattern before we push into the spec.

I'm happy to be swayed either way on this one (i.e. spec vs pattern and candidate for v1.1)

@pwalsh

This comment has been minimized.

Show comment
Hide comment
@pwalsh

pwalsh Jun 21, 2017

Member

I'll do a PR on the spec.

Member

pwalsh commented Jun 21, 2017

I'll do a PR on the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment