Skip to content

fix: _remove_punctuation strips digits causing false palindrome results#2749

Merged
keon merged 1 commit into
keon:mainfrom
faizzyhon:fix/is-palindrome-strips-digits
May 18, 2026
Merged

fix: _remove_punctuation strips digits causing false palindrome results#2749
keon merged 1 commit into
keon:mainfrom
faizzyhon:fix/is-palindrome-strips-digits

Conversation

@faizzyhon

Copy link
Copy Markdown
Contributor

Problem

_remove_punctuation() in algorithms/string/is_palindrome.py used ascii_letters (a-z, A-Z only) to filter characters, silently discarding all digit characters (0-9).

Four of the five palindrome-check variants — is_palindrome_reverse, is_palindrome_two_pointer, is_palindrome_stack, and is_palindrome_deque — all call this helper, so they all returned wrong results for strings containing digits:

# Before this fix:
is_palindrome_reverse("a1b2a")    # True  ← WRONG  (digits stripped → "aba" = palindrome)
is_palindrome_reverse("1a2")      # True  ← WRONG  (digits stripped → "a"   = palindrome)
is_palindrome("a1b2a")            # False ← correct (uses .isalnum() directly)

Fix

Replace the ascii_letters membership test in _remove_punctuation with str.isalnum(), which correctly retains digit characters:

- return "".join(char.lower() for char in text if char in ascii_letters)
+ return "".join(char.lower() for char in text if char.isalnum())

Remove the now-unused from string import ascii_letters import. Update all five functions' docstrings to say "alphanumeric" (was "alphabetic"). Add bound-guards left < right to the two inner while-loops in is_palindrome to prevent an infinite loop on all-punctuation input.

Tests

Added test_is_palindrome_with_digits which exercises all five variants with:

  • "12321"True (valid digit palindrome)
  • "a1b2a"False (not a palindrome once digits are kept)

All 567 existing tests continue to pass.

Updated palindrome functions to consider only alphanumeric characters and ignore case. Improved docstrings for clarity.

@keon keon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

APPROVED. Verified locally: test_string.py 68/68 pass. Good catch — replacing the ascii_letters membership test with str.isalnum() matches the behavior of the standalone is_palindrome function (which already used .isalnum()), so all five variants now agree. The left < right bound-guards on the inner while-loops in is_palindrome are a nice incidental hardening against all-punctuation input.

Note for the maintainer: this overlaps with the is_palindrome portion of #2745.

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.

2 participants