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

Format observations with a comma thousands separator #53

Merged
merged 2 commits into from
Jun 28, 2020

Conversation

MaxGhenis
Copy link
Contributor

Fixes #31 using guidance from https://stackoverflow.com/a/10742904/1840471. This will format the observation count using a locale-aware thousands separator (comma or period).

@MaxGhenis
Copy link
Contributor Author

Friendly ping, I have a few open PRs.

@MaxGhenis
Copy link
Contributor Author

Here's a test:
image

@toobaz
Copy link
Collaborator

toobaz commented Jun 27, 2020

The current version misbehaves on my computer (Italian locale), as both the thousands separator and the decimal separator become a comma: if we follow this path, it must be for all numbers.

But more fundamentally, I don't know other scientific softwares whose output depend on the locale (all those in my typical workflow don't). And I'm not sure I should need to (know and) install some other scientists' locale to exactly reproduce her results. So I'm happy to have the thousands separator, but binding it to the user's locale should at the very minimum be switched by default.

@MaxGhenis MaxGhenis changed the title Format observations with a locale-aware thousands separator Format observations with a comma thousands separator Jun 27, 2020
@MaxGhenis
Copy link
Contributor Author

OK, I removed locale-awareness.

@toobaz toobaz merged commit c9793c6 into StatsReporting:master Jun 28, 2020
@toobaz
Copy link
Collaborator

toobaz commented Jun 28, 2020

Thanks @MaxGhenis !

@MaxGhenis MaxGhenis deleted the thousands branch June 28, 2020 14:52
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.

Add thousands separator to observation count
2 participants