Include the accounting option in fmt_percent() and fmt_number()

[rich-iannone]

The accounting arg/option is only currently available in fmt_currency() but it's useful to have in fmt_percent() and even fmt_number(). Thanks @tareefk for the suggestion on this (for its inclusion in fmt_percent()).

[steveputman]

Hey, @rich-iannone -- when we were dealing with this some time ago, I did submit a PR for this (#306). It's a little out of date since the addition of sigfigs, but is pretty close. I'm happy to try to get the PR passing to see if that's helpful for this, since it does address fmt_percent and fmt_number.

[rich-iannone]

Thank you @steveputman , I think this time we will be more ready for this contribution!

[koppsp]

Regarding this, can we split "accounting" argument back into the way it used to be with one argument for negative value handling and one for currency symbol (or something similar that gives you the flexibility to do one BUT NOT the other)?. There is no way currently to say "wrap negative values in parenthesis ONLY, WITHOUT adding currency symbol as prefix" and vice versa.

Happy to open separate issue, let me know.

[steveputman]

@koppsp I know the formatters have changed over time (and I'm unsure yet whether I've made any breaking changes with respect to all the other functionality @rich-iannone and team have added on the formatters), but--assuming I'm not missing what you're after here--I deal with the issue you face with the choice of formatter call. To use your examples, if you want parentheses but no currency symbol, you would use fmt_number(. . ., accounting = TRUE), whereas if you want the currency symbol but not the parentheses for negative values, you'd use fmt_currency(. . ., accounting = FALSE). Maybe the option should be renamed, because most people (and Excel) associate currency symbols with accounting.

[koppsp]

@steveputman Thanks for the response!

If you are saying that right now I can use fmt_number(. . ., accounting = TRUE), 0.2.2 function does not have this argument.

fmt_number(
  data,
  columns,
  rows = NULL,
  decimals = 2,
  n_sigfig = NULL,
  drop_trailing_zeros = FALSE,
  drop_trailing_dec_mark = TRUE,
  use_seps = TRUE,
  scale_by = 1,
  suffixing = FALSE,
  pattern = "{x}",
  sep_mark = ",",
  dec_mark = ".",
  locale = NULL
)

If you are saying that it could potentially be designed that way, then yeah that would/could/should work. I personally don't see a big difference between a number and a currency besides the currency symbol prefix for currency. I'd recommend making one function and leave it up to the user to "turn the numbers into currency type values" via the function's provided arguments.

[steveputman]

Right: this is still an open issue, and it doesn't work that way now, but that's how it works in the PR referenced higher in the thread (#306).

[rich-iannone]

@steveputman Thanks for being patient and continuing to contribute to your PR for this issue. I promise that the stars will align on this one and your contribution will be reviewed and merged.

We took a half measure to include the accounting option in fmt_number() and fmt_percent() because there was some urgency to at least minimally support it in v0.3.0. For the next release, the focus will be on the alignment of parens problem that you took on.

[steveputman]

@rich-iannone Ha no problem (and not 100% altruistic since it keeps me enjoying the benefits of your new features while still being able to deal with my edge case!)