Refactor plotting functions, improve tests, and update README - #16
Merged
Conversation
Co-authored-by: kpotoh <31224899+kpotoh@users.noreply.github.com>
… notation Co-authored-by: kpotoh <31224899+kpotoh@users.noreply.github.com>
…_mutspec Co-authored-by: kpotoh <31224899+kpotoh@users.noreply.github.com>
Co-authored-by: kpotoh <31224899+kpotoh@users.noreply.github.com>
Fix broken tests, add docstrings, expand test coverage, and improve README
Co-authored-by: kpotoh <31224899+kpotoh@users.noreply.github.com>
Co-authored-by: kpotoh <31224899+kpotoh@users.noreply.github.com>
…-biopython Replace ete3 dependency with custom TreeNode/Tree backed by BioPython
…tree_len to tree_heigth
There was a problem hiding this comment.
Pull request overview
This pull request modernizes PyMutSpec by refactoring phylogenetic tree handling away from ete3, restructuring parts of the package to a src/ layout, and updating plotting utilities, tests, and documentation to match the new APIs and packaging approach.
Changes:
- Introduces a custom BioPython-backed
TreeNode/Treeimplementation and updates code/scripts/tests to use it. - Refactors/relocates plotting utilities and expands test coverage, including parity tests vs
ete3(as a dev-only dependency). - Updates packaging/testing/tooling (pyproject, tox, README, gitignore), and refreshes metadata (LICENSE/CHANGELOG).
Reviewed changes
Copilot reviewed 42 out of 50 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Adds tox test + lint envs across multiple Python versions. |
| tests/test_utils.py | New tests for utility functions (auxiliary/tree/spectra helpers). |
| tests/test_tree_vs_ete3.py | New parity tests comparing custom tree API to ete3. |
| tests/test_tree_annotation.py | Updates tests to renamed get_tree_height. |
| tests/test_plot_spectrum.py | Updates import path for SBS ordering used in plotting tests. |
| tests/test_mutspec_calc.py | Fixes regex escaping in test pattern matching. |
| tests/test_codon_ann.py | Makes expected/observed comparisons tolerant via pytest.approx. |
| tests/data/hum_cytb_ms192syn.csv | Adds test fixture data for 192-component spectra. |
| tests/data/hum_cytb_ms12syn.csv | Adds test fixture data for 12-component spectra. |
| tests/conftest.py | Switches tree fixture from ete3 to custom Tree. |
| src/pymutspec/utils/logging.py | Adds TODO note related to logging configuration handling. |
| src/pymutspec/utils/custom_profile.py | Adds a lightweight profiling decorator helper. |
| src/pymutspec/utils/configs/log_settings.yaml | Adds YAML logging configuration file. |
| src/pymutspec/utils/init.py | Exposes utils helpers via package init. |
| src/pymutspec/io/states.py | Removes tqdm usage in DB preparation loop. |
| src/pymutspec/io/gb.py | Adds GenBank reference parsing helper. |
| src/pymutspec/io/auxiliary.py | Improves regex + documents alignment file discovery helper. |
| src/pymutspec/io/init.py | Exposes IO helpers and state readers via package init. |
| src/pymutspec/draw/spectra.py | Reintroduces/updates spectrum plotting functions and SBS orderings. |
| src/pymutspec/draw/sbs_orders.py | Keeps legacy SBS-order generation notes as commented code. |
| src/pymutspec/draw/init.py | Exposes plotting API; adds legacy alias function. |
| src/pymutspec/constants/sbs.py | Adds SBS constants (12/96/192 + sets + codons). |
| src/pymutspec/constants/init.py | Exports constants from sbs. |
| src/pymutspec/annotation/tree.py | New tree utilities (iter_tree_edges, get_tree_height, etc.). |
| src/pymutspec/annotation/spectra.py | Adds/updates docstrings + strengthens assertions in spectra helpers. |
| src/pymutspec/annotation/phylo_tree.py | Implements custom TreeNode/Tree backed by BioPython Newick parsing. |
| src/pymutspec/annotation/mut.py | Replaces ete3 with custom Tree; improves error handling + regex literals. |
| src/pymutspec/annotation/auxiliary.py | Reintroduces rev-comp + label conversion helpers with docstrings. |
| src/pymutspec/annotation/init.py | Re-exports new tree classes and renamed get_tree_height. |
| src/pymutspec/init.py | Exposes top-level API and defines __version__. |
| scripts/rename_internal_nodes.py | Updates script to use custom tree implementation. |
| scripts/plot_spectra.py | Removes unused imports. |
| scripts/collect_mutations.py | Updates script to use custom tree implementation. |
| scripts/collect_mutations_parallel.py | Updates script to use custom tree implementation + get_tree_height. |
| scripts/calculate_mutspec.py | Removes unused imports and legacy SBS-order import. |
| scripts/alignment2iqtree_states.py | Minor message formatting changes. |
| scripts/1.terminal_genomes2iqtree_format.py | Minor message formatting changes. |
| requirements.dev.txt | Removes legacy dev requirements file. |
| README.md | Major expansion: installation, tox workflow, quickstart, and extended user guide. |
| pyproject.toml | Updates dependencies/extras, pytest config, classifiers, and switches to src package discovery. |
| pymutspec/draw/spectra.py | Removes legacy non-src/ plotting implementation. |
| pymutspec/draw/sbs_orders.py | Removes legacy non-src/ SBS ordering implementation. |
| pymutspec/draw/init.py | Removes legacy non-src/ draw init. |
| pymutspec/annotation/tree.py | Removes legacy non-src/ tree utilities tied to ete3. |
| pymutspec/annotation/auxiliary.py | Removes legacy non-src/ auxiliary helpers. |
| pymutspec/init.py | Removes legacy non-src/ version marker. |
| MANIFEST.in | Removes requirements.txt inclusion. |
| LICENSE | Updates copyright holder line. |
| CHANGELOG.md | Adds 0.0.15 entry describing the refactor and related changes. |
| .gitignore | Adds common build/tox artifacts and local dev files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+15
| commands = | ||
| pytest |
Comment on lines
+57
to
+58
| pip install tox | ||
| tox p |
Comment on lines
+25
to
+28
| try: | ||
| return next(node.iter_ancestors()) | ||
| except BaseException: | ||
| return None |
Comment on lines
+53
to
+67
| discovered_nodes = set() | ||
| discovered_nodes.add(tree.name) | ||
| Q = Queue() | ||
| Q.put(tree) | ||
|
|
||
| while not Q.empty(): | ||
| cur_node = Q.get() | ||
| for child in cur_node.children: | ||
| Q.put(child) | ||
|
|
||
| if cur_node.name not in discovered_nodes: | ||
| discovered_nodes.add(cur_node.name) | ||
| alt_node = cur_node | ||
| ref_node = node_parent(alt_node) | ||
| yield ref_node, alt_node |
Comment on lines
+76
to
+79
| --------- | ||
| tree | ||
| Rooted phylogenetic tree or subtree. Must not be named ``'ROOT'``. | ||
| mode: str |
| rev_comp, transcriptor | ||
| ) | ||
|
|
||
| __version__ = "0.0.15" No newline at end of file |
Comment on lines
+1
to
+4
| from .spectra import plot_mutspec, plot_mutspec12, plot_mutspec192 | ||
|
|
||
| def plot_mutspec192box(*args, **kwargs): | ||
| print("WARNING: the function is removed! Use plot_mutspec192(style='box') instead") |
Contributor
Author
|
@copilot can you translate all suggestions to issue? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant refactoring and modernization of the codebase, focusing on removing the dependency on
ete3in favor of a custom tree implementation, cleaning up legacy or unused code, and updating packaging and dependency management. It also expands Python version compatibility and makes minor improvements to scripts and metadata.Phylogenetic tree refactoring:
ete3runtime dependency with a customTreeNode/Treeimplementation insrc/pymutspec/annotation/phylo_tree.py, and added BioPython-based Newick parsing and an ete3-compatible tree API. This modernizes tree handling and removes a heavy dependency.tests/test_tree_vs_ete3.py) to verify parity with ete3 for tree loading and node/edge iteration.ete3only as adevextra for comparison tests.pymutspec/annotation/tree.pymodule, which contained ete3-based tree utilities.Dependency and packaging updates:
pyproject.tomlto remove version pins fornumpy,pandas, andseaborn, and droppedete3from runtime dependencies. Added several developer tools to thedevextra and expanded supported Python versions up to 3.14.requirements.txtfrom the manifest and cleaned uprequirements.dev.txt. [1] [2]LICENSEto reflect the correct copyright holder.Code cleanup and removal of legacy functions:
pymutspec/annotation/auxiliary.pymodule, which contained legacy label and reverse-complement functions.pymutspec/draw/sbs_orders.pyandpymutspec/draw/__init__.pymodules, and the large plotting modulepymutspec/draw/spectra.py, which depended on the removed auxiliary functions. [1] [2] [3]Minor improvements and fixes:
CHANGELOG.mdand removed the__version__attribute frompymutspec/__init__.py(now managed dynamically). [1] [2]These changes collectively modernize the package, reduce dependencies, and improve maintainability.