fix #2951 Use a thin space to indicate groups of 1000s in html/cli print out

[hossam26644]

Implemented a format_number method in util that accepts a string or a number and returns the number separated by a thin space. Used the format_number for the string representation and the html output of trees, treesequence, and variants

Fixes #2951

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.62%. Comparing base (41f3983) to head (b9fa6c0).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3167   +/-   ##
=======================================
  Coverage   89.62%   89.62%           
=======================================
  Files          28       28           
  Lines       31955    31965   +10     
  Branches     5872     5873    +1     
=======================================
+ Hits        28639    28649   +10     
  Misses       1886     1886           
  Partials     1430     1430           

Flag	Coverage Δ
c-tests	86.66% <ø> (ø)
lwt-tests	80.38% <ø> (ø)
python-c-tests	88.18% <ø> (ø)
python-tests	98.85% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
python/tskit/genotypes.py	99.17% <100.00%> (+0.01%)	⬆️
python/tskit/trees.py	98.87% <ø> (ø)
python/tskit/util.py	99.28% <100.00%> (+0.02%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jeromekelleher]

LGTM. I guess the only downside of this is that people copying these numbers from the text won't get quite what they want. But, it that's just on pretty printing, that's OK?

[benjeffery]

Just needs a fix for the tests. (I added a "fixes" tag to the PR description as it auto-closes the issue on merge)

[benjeffery]

This split is causing the module to fail to import.

[hossam26644]

yes, I also had to change the regex of test_highlevel.py::TestTree::test_str to match the new format; please check that also @benjeffery

[benjeffery]

Looks good on HTML

I'm not sure about the CLI though, as in fixed-width font this looks wrong, and a comma would be better.

[hossam26644]

I can change the separator to a comma for the cli, just confirm this with me before I implement it @benjeffery

[benjeffery]

Yep!

[jeromekelleher]

Agree this is the right approach

[hossam26644]

Is there a way to add squishing different commits to the workflow before merging rather than doing it manually? then if more changes are to be done we wont have to squish again (and I just hate to force push)?

[benjeffery]

Is there a way to add squishing different commits to the workflow before merging rather than doing it manually? then if more changes are to be done we wont have to squish again (and I just hate to force push)?

There's two approaches to this, depending on context.

1. Just push as many commits as you like to the branch of this PR, this lets people review individual commits and is useful as people can see what is changing. We then just squash and force push at the end.

2. git commit --amend and force push to edit the last commit - useful for little changes and linting fixes that aren't useful to see separately.

You should not be scared of force pushing at all, nothing is lost as all the previous states of your branch will be in the reflog. (You can see them with git reflog).

I force push about 50% of the time, it is a normal part of the workflow.

[hossam26644]

@benjeffery another review?

[benjeffery]

Looks good - a couple of nits.

[benjeffery]

Just HTML now.

[benjeffery]

Great, squash and we can merge.