Skip to content

Prevent trimming of trailing 0s#42

Merged
pasahe merged 5 commits intobruigtp:mainfrom
kenkomodo:main
Aug 5, 2025
Merged

Prevent trimming of trailing 0s#42
pasahe merged 5 commits intobruigtp:mainfrom
kenkomodo:main

Conversation

@kenkomodo
Copy link
Contributor

@pasahe Hope all is well! I noticed a bug with the round_digits argument in fc_filter() and fc_split() that caused trailing zeroes to be trimmed (e.g., round_digits = 2 would output "50%" instead of "50.00%"), causing an inconsistent number of values after the decimal.

The fix is super simple - let me know if you have any thoughts.

Cheers!

@pasahe
Copy link
Collaborator

pasahe commented Jul 26, 2025

Hi @kenkomodo!

I hope you are well, too. I'm away for a couple of weeks. I'll take a look at it when I get back.

Many thanks!

Cheers

@pasahe
Copy link
Collaborator

pasahe commented Aug 4, 2025

@kenkomodo,

I took a look, and I’m a bit concerned about backward compatibility with the current behavior — as you mentioned, it currently removes trailing zeros (e.g., 50.00% becomes 50%).

I wonder if we should introduce an additional argument like trim_decimal_zeros, to preserve that behavior when needed. We could default it to FALSE, since it is true that keeping the specified number of decimal places feels more natural.

Let me know what you think!

@kenkomodo
Copy link
Contributor Author

@pasahe Makes sense to me. I can work on making that update, but it will be a couple days.

@kenkomodo
Copy link
Contributor Author

Had a couple extra minutes to put toward it today. I called the argument trim_trailing_zeros rather than trim_decimal_zeros, but let me know if you have strong feelings about it being called something else.

@pasahe
Copy link
Collaborator

pasahe commented Aug 5, 2025

Works perfectly! I noticed that it should be added to the excluded box created in fc_filter() when show_exc = TRUE, but that's okay. I'll add it right away, using the same syntax you used.

Thanks!!

@pasahe pasahe merged commit d452bb3 into bruigtp:main Aug 5, 2025
5 checks passed
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.

2 participants