Support node_labels in write_nexus via TRANSLATE

[kaathewisegit]

Description

Adds a node_labels parameter to write_nexus which allows renaming samples. The trees still use the fast Newick serializer, the remapping is done with a TRANSLATE directive.

Resolves #3435

PR Checklist:

• [?] Tests that fully cover new/changed functionality.
• Documentation including tutorial content if appropriate.
• Changelogs, if there are API changes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.92%. Comparing base (2f26dc6) to head (09b8efb).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3442   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files          37       37           
  Lines       32153    32158    +5     
  Branches     5143     5145    +2     
=======================================
+ Hits        29556    29561    +5     
  Misses       2264     2264           
  Partials      333      333           

Flag	Coverage Δ
C	82.70% <ø> (ø)
c-python	77.34% <ø> (ø)
python-tests	96.40% <100.00%> (+<0.01%)	⬆️
python-tests-no-jit	33.20% <0.00%> (-0.02%)	⬇️

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

Components	Coverage Δ
Python API	98.70% <100.00%> (+<0.01%)	⬆️
Python C interface	91.23% <ø> (ø)
C library	88.86% <ø> (ø)

🚀 New features to boost your workflow:

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

[jeromekelleher]

Looks great, thanks @kaathewisegit! A few minor bits of input above.

It would be nice to verify that a third-party tool like dendropy or BioPython recogises this - would it be possible to check this?

[kaathewisegit]

I've changed write_nexus to raise errors if node_labels is empty or if it contains keys above ts.num_samples.

I also tried adding tests which use BioPython (generate a random ts and see if its NEXUS output is parseable), but it turns out that Bio.Nexus only accepts integer ids for TRANSLATE, so n{i} get rejected as illegal.

https://github.com/biopython/biopython/blob/d76a5581ca3317ec850d72450c458ec4e34e2583/Bio/Nexus/Nexus.py#L1113

[jeromekelleher]

Some comments on the implementation.

It seems like dendropy should support TRANSLATE, so should be easy enough to use that to verify? Just one example is fine, we just want to be sure that some other software can read this.

[kaathewisegit]

I've switched to a lenient translation table construction. And added dendropy parsing tests for all tree sequences from get_example_tree_sequences (except multiple-root and empty ones, as newick serialization doesn't work for those).

[jeromekelleher]

Great! Looks like it's almost ready to go in.

[jeromekelleher]

Stray print

[jeromekelleher]

I don't think the random does much here, I'd just do something like

u = ts.samples()[0]
labels[u] = f"new_node_which_was_{u}"

And then after just check that labels[u] is recognised as a taxon ID by dendropy?

[jeromekelleher]

Suggested change

	:param node_labels: A map of type `{index: name}`. Samples present in
	:param node_labels: A map of type `{node_id: name}`. Samples present in

[jeromekelleher]

Suggested change

	the map will have the given name instead of `n{index}`. Note that
	the map will have the given name instead of `n{node_id}`. Note that

Index is ambigous here, better to be clear