Skip to content

Conversation

@BrianLusina
Copy link
Owner

@BrianLusina BrianLusina commented Nov 10, 2025

Describe your change:

This adds the group anagram code challenge to group anagrams together from a list of provided strings.

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Summary by CodeRabbit

  • New Features

    • Introduced Group Anagrams functionality with both a straightforward and an optimized grouping implementation.
  • Documentation

    • Added a README describing the problem, constraints, and example illustrations.
  • Tests

    • Added a comprehensive test suite covering duplicates, mixed groups, and various edge cases.
@BrianLusina BrianLusina self-assigned this Nov 10, 2025
@BrianLusina BrianLusina added enhancement Algorithm Algorithm Problem Datastructures Datastructures Documentation Documentation Updates Strings labels Nov 10, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 10, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a new "Group Anagrams" feature: README and DIRECTORY entry, two implementations (sorted-key and frequency-count) and a unittest suite; also adds the parameterized test dependency to pyproject.toml.

Changes

Cohort / File(s) Summary
Registry / Docs
DIRECTORY.md, pystrings/anagram/group_anagrams/README.md
Register Group Anagrams in DIRECTORY.md and add README describing the problem, constraints, examples, and notes about output ordering.
Implementation
pystrings/anagram/group_anagrams/__init__.py
Add group_anagrams_naive(strs: List[str]) -> List[List[str]] (sorted-string key, lowercases entries) and group_anagrams(strs: List[str]) -> List[List[str]] (26-element letter-frequency tuple key); both accumulate groups via defaultdict[list].
Tests
pystrings/anagram/group_anagrams/test_group_anagrams.py
Add unittest module using parameterized; includes two parameterized tests and six explicit tests (total 8 cases) exercising duplicates, mixed groups, and edge conditions.
Tooling
pyproject.toml
Add parameterized = "^0.9.0" dependency.

Sequence Diagram(s)

sequenceDiagram participant Caller as Test / User participant Module as group_anagrams module participant Map as Group Map Note over Caller,Module: Input list of strings Caller->>Module: call group_anagrams(strs) alt frequency-count key (new) Module->>Module: compute 26-element freq tuple per string else sorted-key (naive) Module->>Module: sort lowercased string to build key end Module->>Map: append string to group keyed by key Module->>Caller: return list(Map.values()) as List[List[str]] Note over Caller,Module: Order of groups / order within groups not guaranteed 
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to: correctness of lowercasing and non-letter handling in both implementations.
  • Verify frequency-vector mapping covers only lowercase a-z and behaves predictably for unexpected characters.
  • Confirm tests correctly assert group equivalence without relying on ordering.

Poem

🐇 I hop through letters, sorted and counted too,
I bunch them by rhythm, make anagrams true,
One method sorts, another tallies each tune,
Tests clap their paws beneath the moon,
Hooray — grouped words dance in a cheerful rune!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(strings, anagrams): group anagrams' clearly and specifically describes the main change: adding a group anagrams algorithm to the strings/anagrams module.
Description check ✅ Passed The PR description follows the template structure with a clear change description and a comprehensive checklist. All critical items are marked complete including algorithms, type hints, doctests, and reference URLs.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64e4c0e and b1c9167.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • pyproject.toml (1 hunks)
  • pystrings/anagram/group_anagrams/__init__.py (1 hunks)
  • pystrings/anagram/group_anagrams/test_group_anagrams.py (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
pystrings/anagram/group_anagrams/test_group_anagrams.py (2)

2-2: Consider testing both implementations.

The test suite only imports and tests group_anagrams, but the module exports both group_anagrams and group_anagrams_naive. Consider adding test coverage for the naive implementation as well.

You could parameterize the tests to cover both implementations:

import unittest from parameterized import parameterized from . import group_anagrams, group_anagrams_naive class GroupAnagramsTestCase(unittest.TestCase): @parameterized.expand([  (group_anagrams,),  (group_anagrams_naive,),  ]) def test_1(self, func): strs = ["eat", "beat", "neat", "tea"] expected = [["eat", "tea"], ["beat"], ["neat"]] actual = func(strs) self.assertEqual(expected, actual) # ... repeat for other tests

6-46: Tests may be fragile due to exact ordering assumption.

The tests use assertEqual with exact list ordering (e.g., [["eat", "tea"], ["beat"], ["neat"]]). While Python 3.7+ dictionaries maintain insertion order, this creates fragile tests that depend on implementation details rather than semantic correctness.

The README notes "the order in which the output is displayed doesn't matter," but these tests enforce a specific order.

Consider comparing sets instead:

def test_1(self): strs = ["eat", "beat", "neat", "tea"] expected = [["eat", "tea"], ["beat"], ["neat"]] actual = group_anagrams(strs) # Convert to set of frozensets for order-independent comparison self.assertEqual( {frozenset(group) for group in expected}, {frozenset(group) for group in actual} )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e84daec and 64e4c0e.

⛔ Files ignored due to path filters (2)
  • pystrings/anagram/group_anagrams/images/group_anagrams_example_one.png is excluded by !**/*.png
  • pystrings/anagram/group_anagrams/images/group_anagrams_example_two.png is excluded by !**/*.png
📒 Files selected for processing (4)
  • DIRECTORY.md (1 hunks)
  • pystrings/anagram/group_anagrams/README.md (1 hunks)
  • pystrings/anagram/group_anagrams/__init__.py (1 hunks)
  • pystrings/anagram/group_anagrams/test_group_anagrams.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pystrings/anagram/group_anagrams/test_group_anagrams.py (1)
pystrings/anagram/group_anagrams/__init__.py (1)
  • group_anagrams (24-69)
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md

679-679: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


680-680: Unordered list indentation
Expected: 4; Actual: 6

(MD007, ul-indent)

@BrianLusina BrianLusina merged commit 1254903 into main Nov 10, 2025
4 of 7 checks passed
@BrianLusina BrianLusina deleted the feat/group-anagrams branch November 10, 2025 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Algorithm Algorithm Problem Datastructures Datastructures Documentation Documentation Updates enhancement Strings

2 participants