-
Notifications
You must be signed in to change notification settings - Fork 0
Implement package cpf utils
#42
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
📝 WalkthroughWalkthroughThis PR releases cpf-utils v1.0.0 with a unified CpfUtils class that orchestrates CPF validation, formatting, and generation. It re-exports all formatter, generator, and validator components from dependencies (cpf-fmt, cpf-gen, cpf-val), updates dependency versions to >=1.0.0, expands documentation significantly, and replaces legacy tests with targeted unit and integration tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant CpfUtils
participant CpfFormatter
participant CpfGenerator
participant CpfValidator
rect rgb(200, 220, 240)
Note over Client,CpfValidator: CpfUtils (v1.0.0) - Unified Interface
end
Client->>CpfUtils: CpfUtils(formatter_opts?, generator_opts?)
CpfUtils->>CpfUtils: __init__ with optional options
CpfUtils->>CpfFormatter: CpfFormatter(...formatter_options)
CpfUtils->>CpfGenerator: CpfGenerator(...generator_options)
CpfUtils->>CpfValidator: CpfValidator()
Client->>CpfUtils: format(cpf_string, hidden?, ...)
CpfUtils->>CpfFormatter: format(cpf_string, ...)
CpfFormatter-->>CpfUtils: formatted_cpf
CpfUtils-->>Client: formatted_cpf
Client->>CpfUtils: generate(format?, prefix?)
CpfUtils->>CpfGenerator: generate(format, prefix)
CpfGenerator-->>CpfUtils: generated_cpf
CpfUtils-->>Client: generated_cpf
Client->>CpfUtils: is_valid(cpf_string)
CpfUtils->>CpfValidator: is_valid(cpf_string)
CpfValidator-->>CpfUtils: bool
CpfUtils-->>Client: bool
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
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
🧹 Nitpick comments (3)
packages/cpf-utils/README.md (1)
21-21: Optional: Shell command formatting.The
$prefix in shell examples is a style choice. Some markdown linters prefer omitting it to allow easier copy-paste, but this is purely stylistic.packages/cpf-utils/tests/cpf_utils_integration_test.py (1)
18-23: Verify arguments are correctly forwarded to delegated methods.The test confirms that the methods are called, but doesn't verify that the correct arguments are passed through. Integration tests should validate the complete delegation chain.
🔎 Proposed enhancement to verify argument forwarding
assert utils.format("123") == "formatted" assert utils.generate() == "generated" assert utils.is_valid("123") is True - utils.formatter.format.assert_called_once() - utils.generator.generate.assert_called_once() - utils.validator.is_valid.assert_called_once() + utils.formatter.format.assert_called_once_with( + "123", None, None, None, None, None, None, None, None + ) + utils.generator.generate.assert_called_once_with(None, None) + utils.validator.is_valid.assert_called_once_with("123")packages/cpf-utils/src/cpf_utils/cpf_utils.py (1)
37-59: Consider using keyword arguments for better clarity.The method passes all parameters positionally to
self.formatter.format(). While this works, using keyword arguments would make the delegation more explicit and resilient to parameter reordering.🔎 Proposed refactor using keyword arguments
def format( self, cpf_string: str, hidden: bool | None = None, hidden_key: str | None = None, hidden_start: int | None = None, hidden_end: int | None = None, dot_key: str | None = None, dash_key: str | None = None, escape: bool | None = None, on_fail: Callable | None = None, ) -> str: return self.formatter.format( - cpf_string, - hidden, - hidden_key, - hidden_start, - hidden_end, - dot_key, - dash_key, - escape, - on_fail, + cpf_string=cpf_string, + 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, )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/cpf-utils/CHANGELOG.mdpackages/cpf-utils/README.mdpackages/cpf-utils/pyproject.tomlpackages/cpf-utils/src/cpf_utils/__init__.pypackages/cpf-utils/src/cpf_utils/cpf_utils.pypackages/cpf-utils/tests/__init__.pypackages/cpf-utils/tests/conftest.pypackages/cpf-utils/tests/cpf_utils_formatter_test.pypackages/cpf-utils/tests/cpf_utils_generator_test.pypackages/cpf-utils/tests/cpf_utils_init_test.pypackages/cpf-utils/tests/cpf_utils_integration_test.pypackages/cpf-utils/tests/cpf_utils_test.pypackages/cpf-utils/tests/cpf_utils_validator_test.py
💤 Files with no reviewable changes (1)
- packages/cpf-utils/tests/cpf_utils_test.py
🧰 Additional context used
🧬 Code graph analysis (4)
packages/cpf-utils/tests/cpf_utils_validator_test.py (1)
packages/cpf-utils/src/cpf_utils/cpf_utils.py (1)
CpfUtils(9-65)
packages/cpf-utils/tests/cpf_utils_init_test.py (4)
packages/cpf-fmt/src/cpf_fmt/cpf_fmt.py (1)
cpf_fmt(6-29)packages/cpf-fmt/src/cpf_fmt/cpf_formatter_options.py (1)
CpfFormatterOptions(25-145)packages/cpf-gen/src/cpf_gen/cpf_generator_options.py (1)
CpfGeneratorOptions(11-57)packages/cpf-utils/src/cpf_utils/cpf_utils.py (1)
CpfUtils(9-65)
packages/cpf-utils/tests/cpf_utils_integration_test.py (1)
packages/cpf-utils/src/cpf_utils/cpf_utils.py (1)
CpfUtils(9-65)
packages/cpf-utils/tests/cpf_utils_formatter_test.py (1)
packages/cpf-utils/src/cpf_utils/cpf_utils.py (1)
CpfUtils(9-65)
🪛 LanguageTool
packages/cpf-utils/README.md
[style] ~270-~270: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 4348 characters long)
Context: ...tion & Support We welcome contributions! Please see our [Contributing Guidelines...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
packages/cpf-utils/README.md
21-21: Dollar signs used before commands without showing output
(MD014, commands-show-output)
🔇 Additional comments (14)
packages/cpf-utils/CHANGELOG.md (1)
1-16: LGTM! Well-structured changelog.The changelog clearly documents the v1.0.0 release with a comprehensive feature list and proper reference to the README for detailed usage.
packages/cpf-utils/pyproject.toml (3)
74-74: LGTM! Test discovery pattern updated consistently.The pytest configuration now discovers test classes ending with "Test", which aligns with all test classes in this PR.
55-55: Homepage URL is live and accessible.The updated URL
https://cpf-utils.vercel.app/returns HTTP 200 and is functioning correctly.
42-44: All three dependency versions are published and available on PyPI: cpf-fmt 1.0.0 (Dec 8, 2025), cpf-gen 1.0.0 (Dec 10, 2025), and cpf-val 1.0.0 (Dec 5, 2025).packages/cpf-utils/tests/cpf_utils_validator_test.py (1)
6-25: LGTM! Proper delegation testing.The tests correctly verify that
CpfUtils.is_validdelegates to the internal validator with proper argument forwarding for both True and False cases.packages/cpf-utils/tests/conftest.py (1)
7-17: LGTM! Proper test environment setup.The dynamic path augmentation correctly enables importing from local dependency sources during testing, with appropriate existence checks.
packages/cpf-utils/tests/cpf_utils_generator_test.py (1)
6-18: LGTM! Proper delegation testing.The test correctly verifies that
CpfUtils.generateforwards theformatandprefixarguments to the internal generator.packages/cpf-utils/tests/cpf_utils_formatter_test.py (1)
6-31: LGTM! Proper delegation testing with appropriate use of ANY.The test correctly verifies argument forwarding from
CpfUtils.formatto the internal formatter, and appropriately usesANYfor the callback comparison since lambda functions cannot be compared by equality.packages/cpf-utils/tests/cpf_utils_init_test.py (1)
23-30: The.optionsattribute is properly exposed.The
CpfGeneratorclass exposes.optionsas a@property(line 52-54 inpackages/cpf-gen/src/cpf_gen/cpf_generator.py) that returns the_optionsinstance created during initialization, so the test assertion on line 29 is valid.packages/cpf-utils/tests/cpf_utils_integration_test.py (1)
25-33: LGTM!The test properly validates that
__slots__prevents attribute addition and confirms the presence of the expected public attributes.packages/cpf-utils/src/cpf_utils/cpf_utils.py (2)
64-65: LGTM!Clean delegation to the validator with appropriate method signature.
14-35: The dataclass field extraction pattern is correct and compatible with the dependency constructors.All fields in
CpfFormatterOptionsmatch the parameters ofCpfFormatter.__init__(), and all fields inCpfGeneratorOptionsmatch the parameters ofCpfGenerator.__init__(). No runtime failures will occur from parameter mismatches.The asymmetry where
CpfValidatordoes not accept options is intentional—the validator has no configurable options and is designed to be stateless.packages/cpf-utils/src/cpf_utils/__init__.py (2)
1-38: LGTM!The import structure and
__all__definition properly expose the public API surface, including all dependency components and the newCpfUtilsclass.
42-43: LGTM!Creating a default instance allows for convenient usage without requiring explicit instantiation, following a common Python pattern.
| "cpf_val", | ||
| ] | ||
|
|
||
| __version__ = "0.0.0" |
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.
Version mismatch: __version__ should be "1.0.0".
The PR objectives indicate this releases cpf-utils v1.0.0, but __version__ is still set to "0.0.0". This needs to be updated to match the release version.
🔎 Proposed fix
-__version__ = "0.0.0"
+__version__ = "1.0.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| __version__ = "0.0.0" | |
| __version__ = "1.0.0" |
🤖 Prompt for AI Agents
In packages/cpf-utils/src/cpf_utils/__init__.py at line 40, the package version
constant __version__ is still set to "0.0.0"; update that assignment to
"__version__ = \"1.0.0\"" so the module reflects the released cpf-utils v1.0.0
(ensure no other files need synchronized version bumps such as CHANGES/pyproject
if applicable).
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.
@coderabbitai, this valus is dynamicly updated on build & release workflow, so no explicit version is required to be set here.
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.
@juliolmuller, thank you for the clarification! I understand now that the version is managed dynamically by your build and release workflow. This makes sense and is a common practice. I'll note this for future reviews.
✏️ Learnings added
Learnt from: juliolmuller
Repo: LacusSolutions/br-utils-py PR: 42
File: packages/cpf-utils/src/cpf_utils/__init__.py:40-40
Timestamp: 2025-12-25T13:28:29.082Z
Learning: In the br-utils-py repository, the `__version__` variable in package `__init__.py` files is dynamically updated during the build and release workflow, so it should remain as "0.0.0" in the source code.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| def generate(self, format: bool | None = None, prefix: str | None = None) -> str: | ||
| return self.generator.generate(format, prefix) |
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.
🛠️ Refactor suggestion | 🟠 Major
Parameter name format shadows the built-in function.
Using format as a parameter name shadows Python's built-in format() function. While this doesn't cause a runtime error in this context, it's considered a code smell and can lead to confusion. Consider renaming to formatted, format_result, or format_output.
🔎 Proposed fix to avoid shadowing built-in
- def generate(self, format: bool | None = None, prefix: str | None = None) -> str:
- return self.generator.generate(format, prefix)
+ def generate(self, formatted: bool | None = None, prefix: str | None = None) -> str:
+ return self.generator.generate(formatted, prefix)Note: This change would need to be reflected in the generator's API as well, or you'd need to pass it as format=formatted if the underlying API uses format.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/cpf-utils/src/cpf_utils/cpf_utils.py around lines 61-62, the method
parameter named "format" shadows Python's built-in format() function; rename the
parameter to a non-conflicting name like "formatted" or "format_output" and
update the call to self.generator.generate accordingly (either change the
generator's API to accept the new parameter name or call
self.generator.generate(format=formatted) to map the renamed argument), and
update all callers and tests to use the new parameter name to keep the API
consistent.
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.
@coderabbitai, thanyou for the comment. Gowever, this must keep these names for API consistency.
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.
@juliolmuller, understood! Keeping the parameter names consistent across your API makes perfect sense. I appreciate the clarification.
Resolves #9
Summary by CodeRabbit
Release Notes: cpf-utils v1.0.0
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.