Version 2.0.0 by KondratievaOlesya · Pull Request #3 · acg-team/str_mut_signatures

KondratievaOlesya · 2026-01-20T12:44:52Z

[2.0.0] - 2026-01-20

Changed

Updated STR feature construction logic:
- Repeat unit length is now controlled by a single flag (ru_length).
- Repeat unit content is optional and can be specified as either:
  - ru="class" (base composition: AT_only / GC_only / mixed)
  - ru="ru" (full repeat unit sequence).
- If ru is not specified, repeat unit content is not included in features.
Removed old combined ru modes (length, AT) in favor of a clearer, modular design.

Improved

Improved label placement and readability in exposure plots.
Group labels are now positioned more clearly, making multi-group plots easier to interpret.

Documentation

Updated README and CLI help text to reflect the new repeat unit options and feature naming scheme.

…ts; docs

Copilot

Pull request overview

This pull request implements version 2.0.0, a major release with breaking API changes to the STR feature construction logic. The core change refactors how repeat-unit information is included in mutation features by splitting the ru parameter into two independent controls: ru_length (boolean flag) and ru (content mode: None/"class"/"ru").

Changes:

Refactored STR feature construction to use separate ru_length and ru parameters, removing combined modes
Updated repeat-unit classification: renamed "AT_rich"/"non_AT_rich" to "AT_only"/"GC_only"/"mixed"
Improved exposure plot label placement and readability for multi-group visualizations
Added comprehensive NumPy-style docstrings across nmf and extract_tally modules
Added Sphinx documentation infrastructure with RTD theme and GitHub Pages deployment
Updated all tests, CLI, and examples to use the new API

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/str_mut_signatures/init.py	Updated version to 2.0.0
src/str_mut_signatures/cli.py	Updated CLI arguments to support new ru_length/ru parameter split
src/str_mut_signatures/extract_tally/tally.py	Core refactoring: split ru into ru_length + ru, renamed motif_is_at_rich to motif_base_class
src/str_mut_signatures/extract_tally/*.py	Added comprehensive NumPy-style docstrings
src/str_mut_signatures/nmf/*.py	Added comprehensive NumPy-style docstrings and improved plotting
tests/test_tally.py	Updated all tests for new API, added GC_only tests
tests/integration/test_pipeline.py	Updated integration tests to use new API
tests/cli/test_cli_commands.py	Updated CLI tests and version check to 2.0.0
tox.ini	Added gh-actions configuration and py312 testenv for coverage
.github/workflows/ci.yml	Added codecov upload step (has indentation bug)
.github/workflows/docs.yml	New workflow for Sphinx documentation deployment
docs/*	New Sphinx documentation structure
README.rst	Updated examples and documentation for new API
CHANGELOG.md	Added 2.0.0 release notes
pyproject.toml	Added duplicate documentation dependencies
setup.py	Updated development status to Beta

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T12:48:40Z

+  # Upload coverage
+    - name: Upload coverage to Codecov
+      if: matrix.python-version == '3.12'
+      uses: codecov/codecov-action@v5
+      with:
+        files: coverage.xml
+        fail_ci_if_error: true
+        token: ${{ secrets.CODECOV_TOKEN }}


The indentation for the coverage upload step is incorrect. This step should be at the same level as "Test with tox", not nested under it. The current indentation will cause this step to be ignored by the workflow parser.

Copilot · 2026-01-20T12:48:40Z

+    cmap: str = "tab20",
    s: float = 30.0,
 ) -> tuple[pd.DataFrame, np.ndarray, plt.Axes]:
    """
    Run PCA on an NMF result (typically exposures) and plot PC1 vs PC2.

-    This is the main entry point:
-    - extracts a samples x features matrix from `result`
-      (by default `result.exposures`),
-    - computes PCA,
-    - color samples by NMFResult.groups
-    - returns PCA coordinates, variance explained, cluster labels and axes.
+    This is the main entry point for PCA visualization:
+
+    - Extract a samples × features matrix from ``result`` (by default
+      ``result.exposures``).
+    - Compute PCA coordinates.
+    - Color samples by ``result.groups``.
+    - Plot the first two principal components.

    Parameters
    ----------
    result : NMFResult
-        Output of `run_nmf`. Must have `.exposures` as a pandas DataFrame,
-        unless a `matrix` is explicitly passed.
-
-    matrix : pandas.DataFrame or None, default None
-        Optional matrix to use instead of `result.exposures`.
-        Must be samples x features. If None, uses `result.exposures`.
-
-    n_components : int, default 2
+        Output of :func:`run_nmf`. Must provide ``.exposures`` as a
+        :class:`pandas.DataFrame` unless ``matrix`` is explicitly provided.
+    matrix : pandas.DataFrame or None, optional
+        Optional matrix to use instead of ``result.exposures``.
+        Must be samples × features. If ``None``, uses ``result.exposures``.
+    n_components : int, optional
        Number of principal components to compute. Must be >= 2.
-
-    ax : matplotlib.axes.Axes or None, default None
-        Existing axes to plot on. If None, a new figure/axes is created.
-
-    title : str or None, default None
-        Plot title. If None, a default title is generated.
-
-    alpha : float, default 0.8
-        Point transparency.
-
-    s : float, default 30.0
-        Point size.
+        Default is 2.
+    ax : matplotlib.axes.Axes or None, optional
+        Existing axes to plot on. If ``None``, a new figure and axes are created.
+    title : str or None, optional
+        Plot title. If ``None``, a default title is generated.
+    alpha : float, optional
+        Point transparency. Default is 0.8.
+    s : float, optional
+        Point size. Default is 30.0.

    Returns
    -------
    coords : pandas.DataFrame
-        PCA coordinates (PC1, PC2, ...).
-
-    explained_variance_ratio_ : np.ndarray
-        Fraction of variance explained by each component.
-
+        PCA coordinates for each sample (``PC1``, ``PC2``, ...), indexed by sample.
+    explained_variance_ratio_ : numpy.ndarray
+        Fraction of variance explained by each principal component.
    ax : matplotlib.axes.Axes
-        Axes with the PCA scatter plot.
+        Axes containing the PCA scatter plot.
+
+    Raises
+    ------
+    ValueError
+        If ``n_components < 2`` or if the chosen matrix is empty or non-numeric.
    """


Missing cmap parameter documentation. The cmap parameter was added to the function signature at line 130 but is not documented in the Parameters section of the docstring.

Copilot · 2026-01-20T12:48:41Z

+  str_mut_signatures extract \
+    --vcf-dir data/vcfs \
+    --out-matrix counts_len.tsv \
+    --ru-len \


The help text shows '--ru-len' but the actual argument name is '--ru-length'. This inconsistency between the example and the actual CLI flag could confuse users. The argument name at line 131 is 'ru-length', not 'ru-len'.

Suggested change

--ru-len \

--ru-length \

Copilot · 2026-01-20T12:48:41Z

+    "watchdog>=3.0.0",
+    "sphinx",
+    "sphinx-rtd-theme",
+    "myst-parser",
+    "sphinx-argparse"


Duplicate dependencies in dev extras. Lines 53-56 duplicate sphinx, sphinx-rtd-theme, myst-parser, and sphinx-argparse which are already listed in lines 49-50. This duplication is unnecessary and could lead to maintenance issues.

Suggested change

"watchdog>=3.0.0",

"sphinx",

"sphinx-rtd-theme",

"myst-parser",

"sphinx-argparse"

"watchdog>=3.0.0"

Copilot · 2026-01-20T12:48:41Z

        --vcf-dir data/vcfs/ \
        --out-matrix counts_raw.tsv \
-        --ru length \
+        --ru-len \


Inconsistent CLI flag name in the README example. Line 181 shows '--ru-len' but the actual CLI argument defined in cli.py line 131 is '--ru-length'. This should be '--ru-length' to match the actual implementation.

Suggested change

--ru-len \

--ru-length \

Copilot · 2026-01-20T12:48:42Z

    matrix = build_mutation_matrix(
        mutations,
-        ru="length",
+        ru_length="length",


The parameter name value should be "ru_length" not "ru_length="length"". Line 111 shows incorrect syntax where you're assigning the string "length" to ru_length when it should be a boolean True.

Suggested change

ru_length="length",

ru_length=True,

Copilot · 2026-01-20T12:48:43Z

+    # ru_length: 
+    #   include repeat-unit length (LEN{len(motif)})


There's a typo in the comment. 'ru_length:' should be followed by a proper description on the same line, but instead it's on line 99 with no description. The comment structure is inconsistent with the 'ru:' comment block below it.

Suggested change

# ru_length:

# include repeat-unit length (LEN{len(motif)})

# ru_length: include repeat-unit length (LEN{len(motif)})

Copilot · 2026-01-20T12:48:43Z

-            "'ru' (full motif sequence), or 'AT' (AT-rich vs non-AT-rich). "
-            "Default: length"
+            "How to include repeat-unit content in feature labels: "
+            "'class' (base class AT/GC/MX) or "


The help text abbreviates 'mixed' as 'MX', but the actual output uses the full word 'mixed' according to the code in tally.py and test_tally.py. This inconsistency between the help text and actual behavior could confuse users.

Suggested change

"'class' (base class AT/GC/MX) or "

"'class' (base class AT/GC/mixed) or "

… exposure plot

codecov · 2026-01-20T13:02:41Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Copilot

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T13:14:52Z

+                "--ru",
+                "length",


This test is using the deprecated --ru length parameter which no longer exists in version 2.0.0. The parameter should be updated to use --ru-length instead (without a value, since it's a flag), or the test should be updated to use --ru-length and optionally --ru class or --ru ru.

Copilot · 2026-01-20T13:14:52Z

+                "--ru",
+                "length",


This test is using the deprecated --ru length parameter which no longer exists in version 2.0.0. The parameter should be updated to use --ru-length instead (without a value, since it's a flag), or the test should be updated to use --ru-length and optionally --ru class or --ru ru.

Copilot · 2026-01-20T13:14:52Z

-        colors = cmap(norm(plot_df["group"]))
+        # ---- continuous coloring ----
+        cmap_name = cmap if cmap is not None else "viridis"
+        # ---- continuous coloring ----


The comment "# ---- continuous coloring ----" is duplicated on consecutive lines. One of these should be removed.

Suggested change

# ---- continuous coloring ----

Copilot · 2026-01-20T13:14:53Z

+    :param mutations_data: DataFrame containing mutation data.
+    :type mutations_data: pandas.DataFrame
+    :param output_csv: Path to the output CSV file.
+    :type output_csv: str or pathlib.Path


This function uses Sphinx-style docstring format (:param:, :type:), which is inconsistent with the NumPy-style docstrings used throughout the rest of the codebase. For consistency, consider converting this to NumPy-style format with Parameters and Returns sections.

Suggested change

:param mutations_data: DataFrame containing mutation data.

:type mutations_data: pandas.DataFrame

:param output_csv: Path to the output CSV file.

:type output_csv: str or pathlib.Path

Parameters

----------

mutations_data : pandas.DataFrame

DataFrame containing mutation data.

output_csv : str or pathlib.Path

Path to the output CSV file.

Copilot

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T13:25:49Z

            ax.text(
-                x[i], ylim[1], str(group_labels[i]), ha="center", va="bottom", fontsize="x-small"
+                x_start,
+                1.06,  # a bit higher than 1.02 to avoid title/legend collisions
+                str(group_labels[s]),
+                ha="left",      # start text at x_start
+                va="bottom",
+                rotation=30,
+                fontsize="x-small",
+                transform=ax.get_xaxis_transform(),  # x in data, y in axes coords
+                clip_on=False,
            )


The group label placement logic has been improved. The horizontal alignment is set to "left" (line 499) with rotation=30 (line 501), which may cause labels to extend beyond the plot area for groups on the right side. Consider using "center" alignment for better visual balance, especially when n_groups == 1 where the text is placed at x[s] rather than the midpoint.

Copilot · 2026-01-20T13:25:49Z

+
+# -- Options for HTML output -------------------------------------------------
+# https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output
+html_static_path = ['_static']


The html_static_path is set to ['_static'], but this directory may not exist. If the _static directory doesn't exist when Sphinx runs, it will generate a warning. Consider either creating the directory or removing this line if no static files are needed.

Copilot · 2026-01-20T13:25:50Z

@@ -2,7 +2,6 @@ name: Python package

 on:
  - push


The CI workflow has been modified to trigger only on push events (removed pull_request). This means that pull requests will not trigger CI builds unless there's a push to the PR branch. Consider keeping the pull_request trigger to ensure CI runs on all PRs.

Suggested change

- push

push:

pull_request:

Copilot · 2026-01-20T13:25:50Z

+    "sphinx",
+    "sphinx-rtd-theme",
+    "myst-parser",
+    "sphinx-argparse"


Sphinx and documentation dependencies have been added to the dev optional dependencies without version constraints (lines 52-55). This could lead to compatibility issues if breaking changes are introduced in these packages. Consider adding minimum version constraints, especially for sphinx (e.g., sphinx>=7.0.0) to ensure consistency.

Suggested change

"sphinx",

"sphinx-rtd-theme",

"myst-parser",

"sphinx-argparse"

"sphinx>=7.0.0",

"sphinx-rtd-theme>=1.3.0",

"myst-parser>=2.0.0",

"sphinx-argparse>=0.4.0"

Copilot · 2026-01-20T13:25:50Z

+python =
+    3.12: py312


The [gh-actions] section only maps Python 3.12, but the GitHub Actions workflow matrix includes Python versions 3.8-3.13. This means tox-gh-actions won't automatically select the correct environment for Python versions other than 3.12. Either complete the mapping for all versions or remove the [gh-actions] section to rely on tox's default environment selection.

Suggested change

python =

3.12: py312

python =

3.8: py38

3.9: py39

3.10: py310

3.11: py311

3.12: py312

3.13: py313

KondratievaOlesya added 2 commits January 20, 2026 13:20

New option for RU class GC-only/AT-only/mixed; improved labels in plo…

65ccf84

…ts; docs

tox wrong module

594e7be

Copilot AI review requested due to automatic review settings January 20, 2026 12:44

Copilot started reviewing on behalf of KondratievaOlesya January 20, 2026 12:45 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Add default coloring schema for continues and non-continues values in…

5f33080

… exposure plot

KondratievaOlesya requested a review from Copilot January 20, 2026 13:10

Copilot started reviewing on behalf of KondratievaOlesya January 20, 2026 13:11 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

ru length depricated; change to --ru-length

e2ec24e

KondratievaOlesya requested a review from Copilot January 20, 2026 13:22

Copilot started reviewing on behalf of KondratievaOlesya January 20, 2026 13:22 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

KondratievaOlesya merged commit 1d8700e into main Jan 20, 2026
14 checks passed

	# ru_length:
	# include repeat-unit length (LEN{len(motif)})
	# ru_length: include repeat-unit length (LEN{len(motif)})

	"'class' (base class AT/GC/MX) or "
	"'class' (base class AT/GC/mixed) or "

-python =
-.12: py312
+python =
+.8: py38
+.9: py39
+.10: py310
+.11: py311
+.12: py312
+.13: py313

Uh oh!

Conversation

KondratievaOlesya commented Jan 20, 2026

[2.0.0] - 2026-01-20

Changed

Improved

Documentation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jan 20, 2026

Welcome to Codecov 🎉

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects