-
Notifications
You must be signed in to change notification settings - Fork 130
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
feature suggestion: parameter to specify decimal separator on adorn_pct_formatting #451
Comments
Yes, that sounds like a good idea. This may be a blind spot for me as an American - is this for countries where what I know as 99.1% would be written 99,1% ? |
I'm from Europe. |
Whoa that is a lot indeed! Yes, let's add this. I am not sure when I'll be able to implement it. I'd accept a pull request if someone wants to add it, or if none comes, then I'll hopefully add it down the road. I'm trying to think through the implementation...
|
As a reference: |
Just a thought here that I haven't looked into due to lack of time, but isn't this a locale setting for R? Perhaps the function could look to that for info, rather than requiring user input? |
Use adorn_pct_formatting <- function(dat, digits = 1, decimal.mark = getOption("OutDec"), rounding = "half to even", affix_sign = TRUE, ...) |
Ooh nice. Okay I can take a shot at this. Then when I get it on a branch, I'll post here and would love for someone in a comma-using locale to try it out. |
@mgacc0 I have a draft of this as #510. Can you take a look please? Hoping to finalize it in the next week or so to have it ready to go CRAN. I ended up not adding a new argument. Instead it pulls from a user's locale, the value of My thinking was that using the user's locale would do the job virtually all the time, and in rare cases where it doesn't, they can set |
Also @YonghuiDong I see you gave 👍 above, please share if you have any thoughts on the PR linked above |
I haven't heard from any potential user of this feature. I'm going to merge to main branch so it's easier for people to install the dev version, then put out a call on Mastodon and see if I can find anyone to give feedback. |
I am sorry to report that the feature does not seem to be working - when using the Czech locale (we are one of the European troublemakers) I still get the dot as decimal separator; see the output below. Also, on a personal note: it is very common practice for R users from non-English speaking countries to have set English locale as default. There are many reasons for that - such as that you don't really want to google for error messages in some marginal language such as mine, when there are troves of English resources just lying around. Also it is not the bestest of practice to have commas as decimal marks in your actual code; you want dots to make it universal. As a consequence going by the decimal separator from locale can be unreliable. Many of the target users will have their development environment in English, but will be producing locally flavored output.
|
Thank you @jlacko for the user perspective! To check something: would it be a viable workflow for a user who wanted commas in their output tabyl but not the rest of the time to set Based on your feedback I'm thinking there should be an explicit argument in the I don't understand why the locale marker is not getting picked up in your case, but maybe it doesn't matter if that workflow is undesirable anyway. |
Having thought about the issue a bit I believe the approach using For two reasons:
I confess I am liking the approach the more I think about it... |
Alright, so I'd encourage use of In that case I could leave the current code as-is and I would just need to add to the function documentation that users can directly control the printing with |
Yes, that is what I would do. And in encouraging the use of |
I'll implement the documentation in a separate issue, to reflect that the code-writing part of this is done: #520 |
Would you consider of interest adding a parameter "decimal.mark" (or similar) to
adorn_pct_formatting
?The text was updated successfully, but these errors were encountered: