Fix issue #201 - #209
Closed
gdkwxn wants to merge 1 commit into
Closed
Conversation
…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)
There was a problem hiding this comment.
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_letterhelper 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_letterlist.
Suppressed comments (3)
src/pdbcleanresiduestandardizationutils.py:131
get_gap_letterhard-codes a 52-character alphabet and usesdivmod(..., 52), but Issue #201 requires the single-character space to beA-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 asA-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...). | ||
|
|
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.
Issue #201