dlce-eva / python-nexus

python-nexus - Generic nexus (.nex, .trees) reader/writer for python
BSD 2-Clause "Simplified" License
11 stars 3 forks source link

fix minor bug in detranslate #38

Closed SimonGreenhill closed 2 years ago

SimonGreenhill commented 2 years ago

If a tree has no translate block and detranslate was called then an exception would be raised about the mismatch between the number of translate matches.

  1. the translate method, should have been a no-op in this instance. This PR now sets the sentry flag ._been_translated correctly.
  2. the construction of the dummy translate table from the labels in the newick string had a buggy regex which missed the last label i.e. (A,(B,C)); missed C. This PR removes the regex and relies on the newick library to get the leaf labels.
SimonGreenhill commented 2 years ago

(not urgent - no release required yet)

codecov-commenter commented 2 years ago

Codecov Report

Merging #38 (346fabd) into master (32a07ad) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #38   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           59        59           
  Lines         2925      2930    +5     
=========================================
+ Hits          2925      2930    +5     
Impacted Files Coverage Δ
src/nexus/handlers/tree.py 100.00% <100.00%> (ø)
tests/test_handler_TreeHandler.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 32a07ad...346fabd. Read the comment docs.

SimonGreenhill commented 2 years ago

Hmm. tests failing on 3.6 due to clldutils looking for re.Pattern which I think was added in 3.7 (not sure why 3.7 is timing out though). Should we remove support for 3.6?

ImportError while loading conftest '/home/runner/work/python-nexus/python-nexus/tests/conftest.py'.
tests/conftest.py:5: in <module>
    from nexus import NexusReader
/opt/hostedtoolcache/Python/3.6.15/x64/lib/python3.6/site-packages/nexus/__init__.py:4: in <module>
    from nexus.reader import NexusReader
/opt/hostedtoolcache/Python/3.6.15/x64/lib/python3.6/site-packages/nexus/reader.py:13: in <module>
    from nexus.handlers.tree import TreeHandler
/opt/hostedtoolcache/Python/3.6.15/x64/lib/python3.6/site-packages/nexus/handlers/tree.py:5: in <module>
    from clldutils.text import strip_brackets, split_text_with_context
/opt/hostedtoolcache/Python/3.6.15/x64/lib/python3.6/site-packages/clldutils/text.py:130: in <module>
    def replace_pattern(pattern: typing.Union[str, re.Pattern], repl, text: str, flags=0) -> str:
E   AttributeError: module 're' has no attribute 'Pattern'
xrotwang commented 2 years ago

Yes, I think we can remove support for 3.6.

xrotwang commented 2 years ago

Alternatively, we'd have to pin clldutils - but I think that's the worse option.