-
Notifications
You must be signed in to change notification settings - Fork 0
Implement package cpf-fmt
#36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughReplaces a trivial greeting with a complete CPF formatting implementation: adds CpfFormatter, CpfFormatterOptions, exceptions, a configurable cpf_fmt(...) entrypoint, expanded exports and metadata, extensive README, and a full test suite while removing the old simple test. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to focus on during review:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cpf-fmt/pyproject.toml (1)
4-4: Critical: Fix package description.The description incorrectly states "CNPJ (Brazilian personal ID)" when this package is for CPF formatting. CNPJ is a different Brazilian document (company registration).
Apply this diff to fix the description:
-description = "Utility function to format CNPJ (Brazilian personal ID)" +description = "Utility function to format CPF (Brazilian personal ID)"
🧹 Nitpick comments (5)
packages/cpf-fmt/src/cpf_fmt/cpf_fmt.py (1)
18-27: Prefer keyword arguments when constructing CpfFormatter.Using positional arguments makes the code fragile to signature changes and reduces readability. Consider using keyword arguments for clarity and maintainability.
Apply this diff to use keyword arguments:
formatter = CpfFormatter( - hidden, - hidden_key, - hidden_start, - hidden_end, - dot_key, - dash_key, - escape, - on_fail, + hidden=hidden, + hidden_key=hidden_key, + hidden_start=hidden_start, + hidden_end=hidden_end, + dot_key=dot_key, + dash_key=dash_key, + escape=escape, + on_fail=on_fail, )packages/cpf-fmt/tests/cpf_formatter_function_test.py (1)
21-31: Prefer keyword arguments when calling cpf_fmt.Using positional arguments makes the code less readable and more fragile. Consider using keyword arguments for clarity.
Apply this diff:
return cpf_fmt( cpf_string, - hidden, - hidden_key, - hidden_start, - hidden_end, - dot_key, - dash_key, - escape, - on_fail, + hidden=hidden, + hidden_key=hidden_key, + hidden_start=hidden_start, + hidden_end=hidden_end, + dot_key=dot_key, + dash_key=dash_key, + escape=escape, + on_fail=on_fail, )packages/cpf-fmt/tests/cpf_formatter_class_test.py (1)
35-45: Prefer keyword arguments when calling formatter.format.Using positional arguments reduces readability and makes the code more fragile. Consider using keyword arguments.
Apply this diff:
return self.formatter.format( cpf_string, - hidden, - hidden_key, - hidden_start, - hidden_end, - dot_key, - dash_key, - escape, - on_fail, + hidden=hidden, + hidden_key=hidden_key, + hidden_start=hidden_start, + hidden_end=hidden_end, + dot_key=dot_key, + dash_key=dash_key, + escape=escape, + on_fail=on_fail, )packages/cpf-fmt/src/cpf_fmt/cpf_formatter_options.py (2)
24-24: Consider using frozen=True for immutable-like behavior.The dataclass uses
frozen=Falsebut employsobject.__setattr__to bypass normal attribute setting and implements a custom__setattr__for validation. If the intent is to have validated immutability (with controlled mutation),frozen=Truewould be more explicit and prevent accidental direct field mutation.However, this would require adjusting the validation logic. The current design works but is somewhat complex.
96-97: Minor: Redundant set_hidden_range call.The
replace()call creates a new instance, which triggers__post_init__, which already callsset_hidden_range(). The subsequent call on line 97 is redundant sincenew_startandnew_endare already the instance's values at this point.While not harmful, removing this redundancy would slightly improve efficiency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/cpf-fmt/README.md(1 hunks)packages/cpf-fmt/pyproject.toml(3 hunks)packages/cpf-fmt/src/cpf_fmt/__init__.py(1 hunks)packages/cpf-fmt/src/cpf_fmt/cpf_fmt.py(1 hunks)packages/cpf-fmt/src/cpf_fmt/cpf_formatter.py(1 hunks)packages/cpf-fmt/src/cpf_fmt/cpf_formatter_options.py(1 hunks)packages/cpf-fmt/src/cpf_fmt/exceptions.py(1 hunks)packages/cpf-fmt/tests/cpf_fmt_test.py(0 hunks)packages/cpf-fmt/tests/cpf_formatter_class_test.py(1 hunks)packages/cpf-fmt/tests/cpf_formatter_function_test.py(1 hunks)packages/cpf-fmt/tests/cpf_formatter_options_test.py(1 hunks)packages/cpf-fmt/tests/cpf_formatter_test_cases.py(1 hunks)
💤 Files with no reviewable changes (1)
- packages/cpf-fmt/tests/cpf_fmt_test.py
🧰 Additional context used
🧬 Code graph analysis (3)
packages/cpf-fmt/tests/cpf_formatter_options_test.py (2)
packages/cpf-fmt/src/cpf_fmt/cpf_formatter_options.py (2)
DEFAULT_ON_FAIL(19-21)CpfFormatterOptions(25-145)packages/cpf-fmt/src/cpf_fmt/exceptions.py (1)
CpfFormatterHiddenRangeError(41-55)
packages/cpf-fmt/tests/cpf_formatter_class_test.py (3)
packages/cpf-fmt/src/cpf_fmt/cpf_fmt.py (1)
cpf_fmt(6-29)packages/cpf-fmt/src/cpf_fmt/cpf_formatter_options.py (2)
DEFAULT_ON_FAIL(19-21)CpfFormatterOptions(25-145)packages/cpf-fmt/src/cpf_fmt/cpf_formatter.py (1)
CpfFormatter(8-104)
packages/cpf-fmt/tests/cpf_formatter_test_cases.py (4)
packages/cpf-fmt/src/cpf_fmt/cpf_fmt.py (1)
cpf_fmt(6-29)packages/cpf-fmt/src/cpf_fmt/exceptions.py (1)
CpfFormatterHiddenRangeError(41-55)packages/cpf-fmt/tests/cpf_formatter_class_test.py (1)
format(23-45)packages/cpf-fmt/tests/cpf_formatter_function_test.py (1)
format(9-31)
🪛 LanguageTool
packages/cpf-fmt/README.md
[typographical] ~28-~28: Consider adding a comma after ‘Internally’ for more clarity.
Context: ...ing class-based resource from cpf_fmt import CpfFormatter # Or using function-based one f...
(RB_LY_COMMA)
[style] ~94-~94: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3140 characters long)
Context: ...tion & Support We welcome contributions! Please see our [Contributing Guidelines...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
packages/cpf-fmt/README.md
21-21: Dollar signs used before commands without showing output
(MD014, commands-show-output)
🔇 Additional comments (19)
packages/cpf-fmt/README.md (1)
1-111: LGTM! Documentation is comprehensive and well-structured.The README provides clear installation instructions, usage examples for both object-oriented and functional approaches, and detailed formatting options. The examples are helpful and the documentation structure is easy to follow.
packages/cpf-fmt/pyproject.toml (1)
63-63: LGTM! Test class discovery pattern updated appropriately.The change from
["Test*"]to["*Test"]aligns with the test file naming convention used in this PR where test classes have the*Testsuffix (e.g.,CpfFormatterOptionsTest,CpfFormatterClassTest).packages/cpf-fmt/tests/cpf_formatter_options_test.py (1)
16-302: LGTM! Comprehensive test coverage for options handling.The test suite thoroughly covers:
- Default value initialization
- Option persistence and merging
- Boundary validation for hidden ranges
- Type validation for callbacks
- Error cases with appropriate exception handling
The test structure is clear and the coverage is excellent.
packages/cpf-fmt/src/cpf_fmt/__init__.py (1)
1-43: LGTM! Public API exports are well-organized.The package properly exposes:
- Formatter class and function
- Options class with all default constants
- Complete exception hierarchy
The
__all__list ensures explicit control over the public API surface.packages/cpf-fmt/tests/cpf_formatter_class_test.py (1)
47-58: LGTM! Options property access test is well-structured.The test properly verifies that the formatter's options attribute returns a
CpfFormatterOptionsinstance with all default values correctly initialized.packages/cpf-fmt/src/cpf_fmt/cpf_formatter_options.py (3)
37-61: LGTM! Initialization logic properly handles defaults and validation.The
__post_init__method correctly:
- Fills None values with appropriate defaults
- Validates the hidden range
- Ensures on_fail is callable
101-117: LGTM! Range validation is thorough and handles edge cases well.The method properly validates boundaries and defensively auto-swaps start/end if reversed, which is good UX.
119-145: LGTM! Custom setattr provides proper validation.The validation logic ensures:
- Hidden range remains valid when start or end is updated individually
- on_fail cannot be set to None or non-callable values after initialization
The implementation is complex but handles all edge cases correctly.
packages/cpf-fmt/tests/cpf_formatter_test_cases.py (3)
1-22: LGTM!The abstract base class pattern is well-structured for sharing test cases across function and class-based implementations.
24-88: LGTM!Comprehensive coverage of input normalization scenarios including various separator types, mixed characters, and whitespace handling.
153-179: LGTM!Good coverage of error cases including out-of-bounds range values and invalid callback types.
packages/cpf-fmt/src/cpf_fmt/exceptions.py (4)
1-16: LGTM!Well-structured exception with clear message formatting and appropriate attribute storage for debugging.
19-34: LGTM!Consistent structure with the input type error class, providing useful context for debugging option validation failures.
37-55: LGTM!Clear exception hierarchy with informative error messages that include the valid range and provided value.
58-79: LGTM!Smart conditional formatting that provides additional context when the evaluated input differs from the actual input, helping users understand why their input was rejected.
packages/cpf-fmt/src/cpf_fmt/cpf_formatter.py (4)
1-33: LGTM!Clean class initialization with
__slots__for memory efficiency and delegation toCpfFormatterOptionsfor configuration management.
75-84: LGTM!The masking logic correctly handles the inclusive range with proper string slicing and concatenation.
86-99: LGTM!Correct string slicing for the standard CPF format (XXX.XXX.XXX-XX) with configurable separators and optional HTML escaping.
101-104: LGTM!Clean property access to the internal options manager.
Fix as per CodeRabbit review at #36 (comment). Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Fix as per CodeRabbit review at #36 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/cpf-fmt/tests/cpf_formatter_test_cases.py (3)
24-88: Input-normalization tests are comprehensive; consider pytest parametrizationThese tests nicely cover a wide range of messy input forms (dots, dashes, spaces, slashes, mixed characters) all converging to the canonical
"123.456.789-10". To reduce duplication and make future additions cheaper, you could group many of these into a single@pytest.mark.parametrizeover the input/expected pairs while keeping behavior identical.- def test_cpf_without_formatting_formats_to_dots_and_dash(self): - cpf = self.format("12345678910") - - assert cpf == "123.456.789-10", f'Output: "{cpf}", Expected: "123.456.789-10"' - - def test_cpf_with_dashes_formats_to_dots_and_dash(self): - cpf = self.format("123-456-789-10") - - assert cpf == "123.456.789-10" + @pytest.mark.parametrize( + "raw", + [ + "12345678910", + "123-456-789-10", + "123 456 789 10", + "12345678910 ", + " 12345678910", + "1.2.3.4.5.6.7.8.9.1.0", + "1-2-3-4-5-6-7-8-9-1-0", + "1 2 3 4 5 6 7 8 9 1 0", + "12345678910abc", + "123456789 dv 10", + "123/456/789/10", + "123 456 789 / 10", + ], + ) + def test_cpf_normalization_formats_to_dots_and_dash(self, raw: str): + cpf = self.format(raw) + + assert cpf == "123.456.789-10", f'Output: "{cpf}", Expected: "123.456.789-10"'
118-152: Hidden range and custom mask tests give good coverage; you might add explicit edge-boundary casesThe hidden-format tests cover default masking, start-only, end-only, combined ranges, reversed ranges, and custom
hidden_key, which nicely constrains formatter behavior. If you want even stronger guarantees around the allowed[0, 10]index interval, consider adding explicit success cases forhidden_start=10andhidden_end=0to complement the error tests for out-of-range values.
153-179: on_fail and error-path tests look good; consider tightening callback typingThe
on_failbehavior and non-callable error path are both well tested, and the range-error tests align withCpfFormatterHiddenRangeError’s documented bounded integer contract. To help static checkers, you could tighten the type ofon_failfrom a bareCallable | Noneto a more specific signature, e.g.Callable[[str], str] | None, matching how it’s used intest_cpf_formats_to_hidden_format_with_start_rangeand the referenced implementation. Based on relevant code snippets, this should better reflect the intended API.- escape: bool | None = None, - on_fail: Callable | None = None, + escape: bool | None = None, + on_fail: Callable[[str], str] | None = None,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cpf-fmt/tests/cpf_formatter_test_cases.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/cpf-fmt/tests/cpf_formatter_test_cases.py (4)
packages/cpf-fmt/src/cpf_fmt/cpf_fmt.py (1)
cpf_fmt(6-29)packages/cpf-fmt/src/cpf_fmt/exceptions.py (1)
CpfFormatterHiddenRangeError(41-55)packages/cpf-fmt/tests/cpf_formatter_function_test.py (1)
format(9-31)packages/cpf-fmt/tests/cpf_formatter_class_test.py (1)
format(23-45)
🔇 Additional comments (2)
packages/cpf-fmt/tests/cpf_formatter_test_cases.py (2)
8-22: Abstract base test class andformatsignature look solidThe abstract
CpfFormatterTestCasesand itsformat(...)signature line up well with both thecpf_fmtfunction and theCpfFormatter.formatmethod, making it easy to reuse the same cases across implementations without duplication. No issues here.
89-117: Delimiter and escaping behavior are well specifiedThe tests for
dot_key,dash_key, andescape=Trueclearly pin down expected behavior (including HTML-escaping of&and<>into&and<>), which should prevent regressions around delimiter customization and HTML-escaping semantics. No issues spotted.
Fix as per CodeRabbit review at #36 (comment). Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Fix as per CodeRabbit review at #36 (comment).
Resolves #6
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.