Skip to content

Fix issue #201 - #209

Closed
gdkwxn wants to merge 1 commit into
masterfrom
pc_residue
Closed

Fix issue #201#209
gdkwxn wants to merge 1 commit into
masterfrom
pc_residue

Conversation

@gdkwxn

@gdkwxn gdkwxn commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Issue #201

  • Added inner function to 'perform_multiple_alignment' called 'get_gap_letter'
  • added more identifiers in gap_letter (a,b..AA, AB..
  • Added warning message for when a structure has to get additional identifiers (AA, AB)

…etter

- added more identifiers in gap_letter (a,b..AA, AB..
- Added warning message for when a structure has to get additional identifiers (AA, AB)
Copilot AI review requested due to automatic review settings August 2, 2026 05:57

Copilot AI 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.

Pull request overview

This PR addresses Issue #201 by extending the residue renumbering “gap identifier” logic in perform_multiple_alignment so alignments with many gaps can continue past the previous single-letter limit, and it adds user-facing warnings when multi-character identifiers are required.

Changes:

  • Added a get_gap_letter helper to generate gap identifiers beyond the original single-letter list.
  • Introduced tracking (big_structures_warning) to emit a warning when larger gap identifiers are encountered.
  • Updated the alignment-to-residue renumbering loop to use generated identifiers instead of a fixed gap_letter list.
Suppressed comments (3)

src/pdbcleanresiduestandardizationutils.py:131

  • get_gap_letter hard-codes a 52-character alphabet and uses divmod(..., 52), but Issue #201 requires the single-character space to be A-Z + a-z + 0-9 (62). Also, the current implementation can generate 3+ character identifiers (AAA, ...) without ever emitting the requested "ERROR: Too many gaps, check your alignment!".
        alphabet = string.ascii_uppercase + string.ascii_lowercase

        label = ""
        n += 1
        while n > 0:

src/pdbcleanresiduestandardizationutils.py:244

  • The warning trigger uses gap_tracker == 52 (A-Z + a-z), but Issue #201 calls out the single-character space as A-Z + a-z + 0-9 (62). Also, the warning should identify the affected chain(s), not just the structure name, to match the requested "WARNING: Chain xxx contains a lot of gaps. Check alignment!" behavior.
                                if gap_tracker == 52:
                                    big_structures_warning.add(structid_list[I].split("/")[-1])
                                gap_tracker+=1

src/pdbcleanresiduestandardizationutils.py:310

  • The final warning text doesn't match the Issue #201 acceptance text and prints a set, which is non-deterministic in order. Consider printing the exact requested per-chain warning(s) and sorting for stable output.
    if len(big_structures_warning) > 0:
        print(f"\nWARNING: The following structures were affected by large gaps in most of the MSA (>62 positions), "
              f"resulting in residue renumbering that requires two-character alternate identifiers. This may lead "
              f"to problems when opening the structure with molecular visualization software. {big_structures_warning}\n")

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

Comment on lines +225 to +227
for freq in gaps: # accepted gap percentage based on user defined occupancy threshold

# Changing the functionality of gap_letter
Comment on lines +114 to +116
Cycles through all single uppercase letters (A-Z) and lowercase letters
(a-z) before moving into multi-letter combinations (AA, AB, ..., zz, AAA...).

@fatipardo fatipardo closed this Aug 10, 2026
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.

3 participants