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

Better decimal formatting #626

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

drvictorvs
Copy link

@drvictorvs drvictorvs commented Dec 22, 2022

Makes sure that pillar formats decimals according to the decimal mark defined by the user with the OutDec option (as described in help("options")).

I was in doubt whether I should have added any sort of validation to OutDec. I initially wrote it to default to . in EN locales and , in certain European locales, and to accept the OutDec value only if it was a character and length 1. I figured the length was especially important given how pillar deals with screen width. I assumed base would also have such checks. However, out of curiosity I set OutDec to "potato" to see how the base print function would deal with it, and indeed it printed 0.1 as [1] 0potato1. Why should I take that away from users?

So, I didn't add the length requirement, but I imagine you would like to do that, as well as add some sort of type validation. Regardless, the important thing is that pillar respects the OutDecoption.

Best regards.

Makes sure that pillar formats decimals according to the decimal mark defined by the user with the "OutDec" option (as described in help("options")).
@krlmlr
Copy link
Member

krlmlr commented Mar 12, 2023

Thanks.

@hadley: Should our printing depend on getOption("OutDec") ?

@hadley
Copy link
Member

hadley commented Mar 14, 2023

Hmmm, this seems like a change with potentially unexpected consequences, so we’d need to carefully analyse to know whether or not it’s worth.

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 this pull request may close these issues.

None yet

3 participants