Skip to content

v1.6.0; refer to changelog.md - #17

Merged
amckenna41 merged 1 commit into
mainfrom
feature/descriptor-audit-v1.6
Nov 9, 2026
Merged

amckenna41 merged 1 commit into
mainfrom
feature/descriptor-audit-v1.6

Conversation

@amckenna41

@amckenna41 amckenna41 commented Nov 9, 2026 •

Copy link
Copy Markdown
Owner

Summary by cubic

v1.6.0 is a descriptor audit release that corrects several descriptor formulas, data tables, and API structure. Descriptor outputs change in this release; pin protpy<1.6 to reproduce older results.

Breaking changes

  • Descriptor values are no longer rounded; the golden-file baseline is regenerated for v1.6.0 and tests compare with a tolerance.
  • signal.py becomes spectral.py; fft_descriptors and dct_descriptors are deprecated aliases of spectral_descriptors.
  • Several v1.5.0 heuristics (e.g. linguistic_complexity, disorder_propensity) are deprecated and warn; they are removed in 2.0.
  • all_descriptors drops from 10,757 to 1,402 columns, and the classic/extended/all descriptor sets are reorganized.
  • conjoint_triad values are normalized by default; pass normalize=False for raw counts.
  • Removed the varname dependency.

Bug fixes

  • instability_index uses the correct DIWV table (25 of 400 entries were wrong), and quasi_sequence_order composition terms are now fractions that sum to 1.
  • kappa scores single-sign sequences instead of returning NaN and is ~100× faster; ctd_distribution clamps quantile indices and assigns window scores to the centre residue.
  • charge_fractions now reports NCPR, and APAAC, motif-composition, and aggregation edge cases are fixed; the Eisenberg scale is split into eisenberg_arb1984 and eisenberg_jmb1984.

Written for commit 79be27d. Summary will update on new commits.

View guided diff Turn on auto-fix

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31 issues found across 60 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/test_spectral.py">

<violation number="1" location="tests/test_spectral.py:27">
P3: This check does not detect the claimed `protpy.signal` removal because it imports the unrelated top-level stdlib module. Import `protpy.signal` and assert that it is unavailable, while retaining the stdlib check if desired.</violation>
</file>

<file name=".gitignore">

<violation number="1" location=".gitignore:139">
P3: `/planning` ignores the whole directory, but `planning/BLOCKED.md` is still tracked. New files added under `planning/` will be silently skipped by `git add`. If the directory is meant to stay local, untrack `planning/BLOCKED.md`; otherwise drop this rule.</violation>
</file>

<file name="examples/example_datasets/README.md">

<violation number="1" location="examples/example_datasets/README.md:118">
P2: The new shape `261 × 1,403` describes `descriptors_thermostability.csv`, but that file is not in the repository and this PR does not add it. Either commit the regenerated CSV with this PR or note that the shape comes from an out-of-tree run, so the documented numbers can be verified.</violation>
</file>

<file name="CHANGELOG.md">

<violation number="1" location="CHANGELOG.md:26">
P3: The bullet omits the tab and carriage-return prefixes that the code also escapes, so it understates the CSV escaping. Add them to the list, and note that column names are escaped too.</violation>

<violation number="2" location="CHANGELOG.md:49">
P3: These deprecation warnings are user-visible, but this bullet sits under 'Removed (internal clean-up; no output changes)'. Move it to the '### Deprecated (removed in 2.0)' section so users can find it.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:60">
P2: `boman_index` is not an alias of `gravy`. It is `-gravy(scale="RADA880101")`, a different scale and a negated value. Describe it as a derived index, or drop the alias note.</violation>

<violation number="2" location="README.md:71">
P2: `distance_pair` is a separate descriptor, not an alias of `cksaap`. It uses cluster profiles (cp14) at distances 1..3 and returns 602 columns, not 2400. Remove the alias note and describe it as its own descriptor.</violation>
</file>

<file name="docs/index.rst">

<violation number="1" location="docs/index.rst:98">
P2: This sentence is inaccurate: `dde`, `autocorrelation` and `aa_scale_descriptors` are opt-in and are not added by `extended` or `all`, and `ctd` is already in the classic set. Reword it to say most of the listed descriptors are added, and name the opt-in ones that need `include=[...]`.</violation>
</file>

<file name="planning/BLOCKED.md">

<violation number="1" location="planning/BLOCKED.md:17">
P2: The cited audit document `planning/protpy descriptor value audit.md` is not in the repository, so this link is dead and the "Not planned" rationale cannot be checked. Add the audit file in this PR or drop the reference.</violation>
</file>

<file name="docs/api.rst">

<violation number="1" location="docs/api.rst:4">
P3: This sentence claims every module documents its `__all__`, but `protpy.encode` has no `__all__`, so its `:members:` lists all public functions, including `read_sequences_from_file`. Qualify the sentence to name `encode` as the exception, or add an `__all__` to `encode.py`.</violation>
</file>

<file name="protpy/data/residue_tables.json">

<violation number="1" location="protpy/data/residue_tables.json:757">
P3: The provenance count is off by one: comparing against the old v1.5 table gives 24 differing entries, not 25. Correct '25' to '24' here, in CHANGELOG.md, and in the instability_index docstring.</violation>
</file>

<file name="protpy/README.md">

<violation number="1" location="protpy/README.md:147">
P3: This example reassigns `charge`, so the first result is overwritten. Use a different variable name for the per-Da result, such as `charge_mass`.</violation>
</file>

<file name="tests/README.md">

<violation number="1" location="tests/README.md:27">
P3: This description overstates the short-sequence coverage: the test exercises lengths 1, 2, and 8 only, not the full 1–8 range. Say which representative lengths are covered or add the missing lengths.</violation>

<violation number="2" location="tests/README.md:31">
P3: This snapshot is not the default output of every registered descriptor: `aa_scale_descriptors` requires a scale and the test pins it to Atchley factors. Qualify this description so readers do not mistake the generic-scale case for a default-compatibility guarantee.</violation>
</file>

<file name="tests/_compare.py">

<violation number="1" location="tests/_compare.py:9">
P2: This comparison is not limited to `atol`: NumPy's default relative tolerance widens the allowed error for larger descriptor values, so regressions beyond the documented tolerance can pass. Pass `rtol=0` to enforce the requested absolute tolerance.</violation>
</file>

<file name="tests/test_protpy.py">

<violation number="1" location="tests/test_protpy.py:59">
P2: The deprecated API test omits the two exported composition helpers `correlation_function` and `amphiphilic_correllation_function`, so a regression in either public compatibility function can pass this suite. Add both to dedicated warning checks using their `(Ri, Rj)` arguments rather than the sequence-based calls used for descriptors.</violation>
</file>

<file name="protpy/spectral.py">

<violation number="1" location="protpy/spectral.py:125">
P2: The new `n >= L` check makes the deprecated `dct_descriptors` reject `n_coeffs == len(sequence)`, which the pre-PR code accepted. Add this boundary change to the CHANGELOG deprecation/changed notes, since it is not listed there. The error message also says `n`, not `n_coeffs`, which is confusing for callers of the deprecated wrapper.</violation>
</file>

<file name="tests/test_structural.py">

<violation number="1" location="tests/test_structural.py:123">
P3: This fill-value regression check covers only COREscore, so `PLAAC_LLR` could remain NaN and the test would still pass. Assert that the filled result contains `0.0` in both undefined score columns.</violation>

<violation number="2" location="tests/test_structural.py:130">
P3: This regression check covers only Parker; Janin’s four outputs can regress without failing the suite. Compare the Janin slice with `hydrophobicity_profile(..., scale="janin", window=7)` as well.</violation>
</file>

<file name="protpy/autocorrelation.py">

<violation number="1" location="protpy/autocorrelation.py:82">
P2: The docstring says protpy scale names are accepted, but multi-dimensional scales such as `vhse` crash here with a numpy broadcast error. Either restrict `properties` to single-valued scales and raise a clear `ValueError` for the others, or drop "or protpy scale names" from the docstring.</violation>
</file>

<file name="protpy/_helpers.py">

<violation number="1" location="protpy/_helpers.py:104">
P2: The eisenberg deprecation warning is attributed to protpy's internal `_property_matrix` frame, so default Python filters hide it from users. Emit it with a stacklevel that reaches the caller's code, or warn from the public entry points. The experimental-scale `UserWarning` has the same wrong stacklevel.</violation>
</file>

<file name="protpy/structural.py">

<violation number="1" location="protpy/structural.py:213">
P3: `_fold_index_profile` assigns even-length windows off-centre, since `(window - 1) // 2` leaves one extra residue on the right. `charge_hydropathy(window=...)` also accepts even values without error, while `aggrescan` rejects them. Reject even windows here, or document that they are off-centre.</violation>

<violation number="2" location="protpy/structural.py:682">
P3: The deprecation message names only the Parker replacement, but this function also returns the JANIN_* columns. Name the Janin replacement too so users can migrate both profiles.</violation>
</file>

<file name="protpy/sequence_order.py">

<violation number="1" location="protpy/sequence_order.py:131">
P2: `sequence_order_coupling_number_` silently returns 0.0 when `d >= len(sequence)`, which breaks the single lag policy (`_check_lag`) that the rest of the module now raises on. Validate `d` with `_check_lag` and drop the 0.0 fallback.</violation>

<violation number="2" location="protpy/sequence_order.py:132">
P3: This computes every lag from 1 to d and keeps only the last one, so the cost is O(L*d) per call. Compute tau_d directly from `matrix[idx[:-d], idx[d:]].sum()` to keep the cost at O(L).</violation>
</file>

<file name="protpy/encode.py">

<violation number="1" location="protpy/encode.py:519">
P2: Rerunning a dataset against a pre-v1.6 CSV leaves raw formula-like identifiers in place because replacement matches only the newly escaped form, then appends a second row. Match both legacy raw identifiers and escaped identifiers when removing replacements.</violation>

<violation number="2" location="protpy/encode.py:718">
P3: The new FASTA branch silently accepts invalid `id_col` and `seq_col` types instead of raising the documented `TypeError`, unlike the delimited-file path. Validate both arguments before constructing the DataFrame.</violation>

<violation number="3" location="protpy/encode.py:718">
P2: FASTA parsing silently drops the identifiers when `id_col` equals `seq_col`, including `id_col="sequence"` with the default sequence name. Reject colliding column names or construct the DataFrame with distinct names before returning it.</violation>
</file>

<file name="protpy/scales.py">

<violation number="1" location="protpy/scales.py:139">
P3: `mode="position"` still rejects `domains` values that are irrelevant to positional output. Apply the `domains > len(sequence)` check only for `mode="mean"`, so positional encodings can ignore the domain parameter as documented.</violation>
</file>

<file name="protpy/ctd.py">

<violation number="1" location="protpy/ctd.py:269">
P2: `ctd` supports `parts`, but the documented `ctd_` alias does not, so `ctd_(sequence, parts="C")` raises `TypeError`. Add `parts` to `ctd_` and forward it to `ctd`.</violation>
</file>

<file name="protpy/composition.py">

<violation number="1" location="protpy/composition.py:106">
P2: The compatibility re-export is not included in `composition.__all__`, so `from protpy.composition import *` no longer imports `shannon_entropy`. Add `"shannon_entropy"` to `__all__`.</violation>
</file>

Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Turn on auto-fix | Re-trigger cubic

| Property | Value |
|----------|-------|
| **Shape** | 260 rows × 10,758 columns |
| **Shape** | 261 rows × 1,403 columns |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new shape 261 × 1,403 describes descriptors_thermostability.csv, but that file is not in the repository and this PR does not add it. Either commit the regenerated CSV with this PR or note that the shape comes from an out-of-tree run, so the documented numbers can be verified.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At examples/example_datasets/README.md, line 118:

<comment>The new shape `261 × 1,403` describes `descriptors_thermostability.csv`, but that file is not in the repository and this PR does not add it. Either commit the regenerated CSV with this PR or note that the shape comes from an out-of-tree run, so the documented numbers can be verified.</comment>

<file context>
@@ -111,13 +111,13 @@ data = protpy.read_sequences_from_file(
 | Property | Value |
 |----------|-------|
-| **Shape** | 260 rows × 10,758 columns |
+| **Shape** | 261 rows × 1,403 columns |
 | **First column** | `name` — sequence identifier matching `thermostability.txt` |
-| **Remaining columns** | All protpy descriptor features (10,757 features with default parameters) |
</file context>

Comment thread README.md
* **cksaagp** (CKSAAGP) — alias of `cksaap`
* **cksaap** (CKSAAP)
* **asdc** (ASDC)
* **distance_pair** (DP) — alias of `cksaap`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: distance_pair is a separate descriptor, not an alias of cksaap. It uses cluster profiles (cp14) at distances 1..3 and returns 602 columns, not 2400. Remove the alias note and describe it as its own descriptor.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At README.md, line 71:

<comment>`distance_pair` is a separate descriptor, not an alias of `cksaap`. It uses cluster profiles (cp14) at distances 1..3 and returns 602 columns, not 2400. Remove the alias note and describe it as its own descriptor.</comment>

<file context>
@@ -46,87 +46,154 @@ This software is aimed at any researcher or developer using protein sequence/str
+* **cksaagp** (CKSAAGP) — alias of `cksaap`
+* **cksaap** (CKSAAP)
+* **asdc** (ASDC)
+* **distance_pair** (DP) — alias of `cksaap`
+* **tripeptide_composition** (TPComp) — alias of `kmer_composition`
+* **kmer_composition** (kmer)
</file context>
Suggested change
* **distance_pair** (DP) — alias of `cksaap`
* **distance_pair** (DP) — cluster-pair distance descriptor (cp14, rng=3)

Comment thread README.md
* **molecular_weight** (MW)
* **aliphatic_index** (AI)
* **extinction_coefficient** (ExtCoeff)
* **boman_index** (BI) — alias of `gravy`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: boman_index is not an alias of gravy. It is -gravy(scale="RADA880101"), a different scale and a negated value. Describe it as a derived index, or drop the alias note.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At README.md, line 60:

<comment>`boman_index` is not an alias of `gravy`. It is `-gravy(scale="RADA880101")`, a different scale and a negated value. Describe it as a derived index, or drop the alias note.</comment>

<file context>
@@ -46,87 +46,154 @@ This software is aimed at any researcher or developer using protein sequence/str
+* **molecular_weight** (MW)
+* **aliphatic_index** (AI)
+* **extinction_coefficient** (ExtCoeff)
+* **boman_index** (BI) — alias of `gravy`
+* **charge_distribution** (ChargeDist)
+* **hydrophobic_moment** (μH)
</file context>
Suggested change
* **boman_index** (BI) — alias of `gravy`
* **boman_index** (BI) — negated GRAVY on the RADA880101 scale

Comment thread docs/index.rst

v1.6.0 merged most of the v1.5.0 additions into these family functions and deprecated the rest (a
``DeprecationWarning`` is raised; they are removed in 2.0). The classic ``all_descriptors`` set is about
1,400 columns; ``descriptor_set="extended"`` or ``"all"`` add the descriptors above. See :doc:`usage` and

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This sentence is inaccurate: dde, autocorrelation and aa_scale_descriptors are opt-in and are not added by extended or all, and ctd is already in the classic set. Reword it to say most of the listed descriptors are added, and name the opt-in ones that need include=[...].

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/index.rst, line 98:

<comment>This sentence is inaccurate: `dde`, `autocorrelation` and `aa_scale_descriptors` are opt-in and are not added by `extended` or `all`, and `ctd` is already in the classic set. Reword it to say most of the listed descriptors are added, and name the opt-in ones that need `include=[...]`.</comment>

<file context>
@@ -68,70 +68,35 @@ Sequence Order Descriptors (5)
+
+v1.6.0 merged most of the v1.5.0 additions into these family functions and deprecated the rest (a
+``DeprecationWarning`` is raised; they are removed in 2.0). The classic ``all_descriptors`` set is about
+1,400 columns; ``descriptor_set="extended"`` or ``"all"`` add the descriptors above. See :doc:`usage` and
+:doc:`descriptors`, including the table of deprecated functions.
 
</file context>
Suggested change
1,400 columns; ``descriptor_set="extended"`` or ``"all"`` add the descriptors above. See :doc:`usage` and
1,400 columns; ``descriptor_set="extended"`` or ``"all"`` add most of the descriptors above (``dde``, ``autocorrelation`` and ``aa_scale_descriptors`` are opt-in via ``include=[...]``). See :doc:`usage` and

Comment thread planning/BLOCKED.md

## Not planned

Closed by the v1.6.0 descriptor value audit (`planning/protpy descriptor value audit.md`): almost every item would add another view of information protpy already has.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The cited audit document planning/protpy descriptor value audit.md is not in the repository, so this link is dead and the "Not planned" rationale cannot be checked. Add the audit file in this PR or drop the reference.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At planning/BLOCKED.md, line 17:

<comment>The cited audit document `planning/protpy descriptor value audit.md` is not in the repository, so this link is dead and the "Not planned" rationale cannot be checked. Add the audit file in this PR or drop the reference.</comment>

<file context>
@@ -1,8 +1,36 @@
+
+## Not planned
+
+Closed by the v1.6.0 descriptor value audit (`planning/protpy descriptor value audit.md`): almost every item would add another view of information protpy already has.
+
+| Item | Reason |
</file context>

Comment thread protpy/structural.py
sequence. Dataframe will be of the shape 1 x 8, with the columns
PARKER_mean, PARKER_max, PARKER_min, PARKER_std and the same for JANIN.
"""
_warn_deprecated("antigenicity_surface", 'hydrophobicity_profile(sequence, scale="parker", window=7)')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The deprecation message names only the Parker replacement, but this function also returns the JANIN_* columns. Name the Janin replacement too so users can migrate both profiles.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At protpy/structural.py, line 682:

<comment>The deprecation message names only the Parker replacement, but this function also returns the JANIN_* columns. Name the Janin replacement too so users can migrate both profiles.</comment>

<file context>
@@ -599,6 +679,7 @@ def antigenicity_surface(sequence: str, window: int = 7) -> pd.DataFrame:
         sequence. Dataframe will be of the shape 1 x 8, with the columns
         PARKER_mean, PARKER_max, PARKER_min, PARKER_std and the same for JANIN.
     """
+    _warn_deprecated("antigenicity_surface", 'hydrophobicity_profile(sequence, scale="parker", window=7)')
     #validate input protein sequence
     sequence = _validate_sequence(sequence)
</file context>

Comment thread protpy/structural.py
h = _sliding_windows([(kyte_doolittle_[aa] + 4.5) / 9 for aa in sequence], window)
r = np.abs(_sliding_windows(_charge_vector(sequence), window))
fi = 2.785 * h - r - 1.151
left = (window - 1) // 2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: _fold_index_profile assigns even-length windows off-centre, since (window - 1) // 2 leaves one extra residue on the right. charge_hydropathy(window=...) also accepts even values without error, while aggrescan rejects them. Reject even windows here, or document that they are off-centre.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At protpy/structural.py, line 213:

<comment>`_fold_index_profile` assigns even-length windows off-centre, since `(window - 1) // 2` leaves one extra residue on the right. `charge_hydropathy(window=...)` also accepts even values without error, while `aggrescan` rejects them. Reject even windows here, or document that they are off-centre.</comment>

<file context>
@@ -171,23 +185,91 @@ def _mean_hydropathy(sequence: str) -> float:
+    h = _sliding_windows([(kyte_doolittle_[aa] + 4.5) / 9 for aa in sequence], window)
+    r = np.abs(_sliding_windows(_charge_vector(sequence), window))
+    fi = 2.785 * h - r - 1.151
+    left = (window - 1) // 2
+    return np.concatenate([np.full(left, fi[0]), fi, np.full(window - 1 - left, fi[-1])])
 
</file context>

Comment thread protpy/sequence_order.py
return round(tau, 3)
_matrix_names(distance_matrix)
d = _validate_int("d", d, 1)
return float(_socn(sequence, d, distance_matrix)[-1]) if d < len(sequence) else 0.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This computes every lag from 1 to d and keeps only the last one, so the cost is O(L*d) per call. Compute tau_d directly from matrix[idx[:-d], idx[d:]].sum() to keep the cost at O(L).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At protpy/sequence_order.py, line 132:

<comment>This computes every lag from 1 to d and keeps only the last one, so the cost is O(L*d) per call. Compute tau_d directly from `matrix[idx[:-d], idx[d:]].sum()` to keep the cost at O(L).</comment>

<file context>
@@ -73,267 +122,163 @@ def sequence_order_coupling_number_(sequence: str, d: int = 1, distance_matrix:
-    return round(tau, 3)
+    _matrix_names(distance_matrix)
+    d = _validate_int("d", d, 1)
+    return float(_socn(sequence, d, distance_matrix)[-1]) if d < len(sequence) else 0.0
 
 def sequence_order_coupling_number(sequence: str, lag: int = 30,
</file context>

Comment thread protpy/encode.py
ids, sequences = _parse_fasta(filepath)
if id_col is None:
return sequences
return pd.DataFrame({id_col if isinstance(id_col, str) else "id": ids,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The new FASTA branch silently accepts invalid id_col and seq_col types instead of raising the documented TypeError, unlike the delimited-file path. Validate both arguments before constructing the DataFrame.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At protpy/encode.py, line 718:

<comment>The new FASTA branch silently accepts invalid `id_col` and `seq_col` types instead of raising the documented `TypeError`, unlike the delimited-file path. Validate both arguments before constructing the DataFrame.</comment>

<file context>
@@ -729,7 +712,11 @@ def read_sequences_from_file(
+        ids, sequences = _parse_fasta(filepath)
+        if id_col is None:
+            return sequences
+        return pd.DataFrame({id_col if isinstance(id_col, str) else "id": ids,
+                             seq_col if isinstance(seq_col, str) else "sequence": sequences})
     elif ext == ".txt":
</file context>

Comment thread protpy/scales.py
sequence = _validate_sequence(sequence)
if scale not in factor_scales_:
raise ValueError(f"Invalid scale '{scale}'. Valid scales are: {list(factor_scales_)}.")
if mode not in ("mean", "position"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: mode="position" still rejects domains values that are irrelevant to positional output. Apply the domains > len(sequence) check only for mode="mean", so positional encodings can ignore the domain parameter as documented.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At protpy/scales.py, line 139:

<comment>`mode="position"` still rejects `domains` values that are irrelevant to positional output. Apply the `domains > len(sequence)` check only for `mode="mean"`, so positional encodings can ignore the domain parameter as documented.</comment>

<file context>
@@ -110,32 +119,39 @@ def aa_scale_descriptors(sequence: str, scale: str, domains: int = 1) -> pd.Data
     sequence = _validate_sequence(sequence)
     if scale not in factor_scales_:
         raise ValueError(f"Invalid scale '{scale}'. Valid scales are: {list(factor_scales_)}.")
+    if mode not in ("mean", "position"):
+        raise ValueError(f"Invalid mode '{mode}'. Valid modes are: ['mean', 'position'].")
     domains = _validate_int("domains", domains, 1)
</file context>

@amckenna41
amckenna41 merged commit a384b44 into main Nov 9, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant