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

More intuitive bool-to-string casting #4666

Merged
merged 2 commits into from
Aug 9, 2023
Merged

Conversation

fsdvh
Copy link
Contributor

@fsdvh fsdvh commented Aug 8, 2023

Which issue does this PR close?

More intuitive values for default bool-to-string conversion

Rationale for this change

It's non very obvious that bool will be converted to 1 or 0, I imagine that the main case for such casting would be comparison with string, which I guess should be done using a direct string representation of bool like true & false

What changes are included in this PR?

The default behavior for bool casting to string

Are there any user-facing changes?

Yes

@fsdvh fsdvh changed the title use more intuitive bool to string casting More intuitive bool to string casting Aug 8, 2023
@fsdvh fsdvh changed the title More intuitive bool to string casting More intuitive bool-to-string casting Aug 8, 2023
@github-actions github-actions bot added the arrow Changes to the arrow crate label Aug 8, 2023
@tustvold tustvold added the api-change Changes to the arrow API label Aug 8, 2023
@tustvold
Copy link
Contributor

tustvold commented Aug 8, 2023

I have confirmed this is also consistent with ArrayFormatter. Perhaps we could even just remove the special case for boolean to string casting, and use ArrayFormatter as is done for other array types? i.e. update it to use value_to_string instead of a custom iterator

@tustvold tustvold merged commit 92d8ee6 into apache:master Aug 9, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change Changes to the arrow API arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants