-
Notifications
You must be signed in to change notification settings - Fork 24
Update banks #179
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
Update banks #179
Conversation
WalkthroughThis change updates the README.md file to correct a minor heading formatting issue and adds a new section describing the use of the Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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)
clabe/banks.py (1)
127-133: Alphabetical-order invariant still respected – consider automating the check
'Cashi Cuenta'correctly slots between'Caja Pop Mexica'and'CB Intercam', so the manual edit preserves the “alphabetical by bank name” rule stated in the comment.To avoid human error for future edits, a tiny pytest asserting alphabetical order would be useful:
def test_bank_names_alphabetical(): names = list(BANK_NAMES.values()) assert names == sorted(names, key=str.casefold)README.md (2)
125-129: Add the missing comma before “pero”
Spanish style guides require a comma before the conjunction “pero”.-**Nuevos participantes**: Instituciones que aparecen en Banxico pero no están en el paquete +**Nuevos participantes**: Instituciones que aparecen en Banxico, pero no están en el paquete
140-144: Minor punctuation tweak
For parallelism, add a period at the end of the last bullet.-**CHANGES**: Cambios de nombre que requieren actualización +**CHANGES**: Cambios de nombre que requieren actualización.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)clabe/banks.py(1 hunks)clabe/version.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`**/*.py`: Enforce Relative Imports for Internal Modules
Ensure that any import...
**/*.py: Enforce Relative Imports for Internal ModulesEnsure that any imports referencing internal modules use relative paths. However, if modules reside in the main module directories (for example /src or /library_or_app_name) —and relative imports are not feasible—absolute imports are acceptable. Additionally, if a module is located outside the main module structure (for example, in /tests or /scripts at a similar level), absolute imports are also valid.
Examples and Guidelines:
- If a module is in the same folder or a subfolder of the current file, use relative imports. For instance: from .some_module import SomeClass
- If the module is located under /src or /library_or_app_name and cannot be imported relatively, absolute imports are allowed (e.g., from library_or_app_name.utilities import helper_method).
- If a module is outside the main module directories (for example, in /tests, /scripts, or any similarly placed directory), absolute imports are valid.
- External (third-party) libraries should be imported absolutely (e.g., import requests).
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
clabe/version.pyclabe/banks.py
`**/*.py`:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code:...
**/*.py:
Rule: Enforce Snake Case in Python Backend
- New or Modified Code: Use snake_case for all variables, functions, methods, and class attributes.
- Exceptions (Pydantic models for API responses):
- Primary fields must be snake_case.
- If older clients expect camelCase, create a computed or alias field that references the snake_case field.
- Mark any camelCase fields as deprecated or transitional.
Examples
Invalid:
class CardConfiguration(BaseModel): title: str subTitle: str # ❌ Modified or new field in camelCaseValid:
class CardConfiguration(BaseModel): title: str subtitle: str # ✅ snake_case for new/modified field @computed_field def subTitle(self) -> str: # camelCase allowed only for compatibility return self.subtitleAny direct use of camelCase in new or updated code outside of these exceptions should be flagged.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
clabe/version.pyclabe/banks.py
`**/*.py`: Use try/except for concise error handling when accessing nested dicti...
**/*.py: Use try/except for concise error handling when accessing nested dictionary keys:try: can_ignore_error = data['error']['code'] in ignore_error_codes except KeyError: can_ignore_error = False❌ Avoid Verbose Chained Conditionals:
can_ignore_error = ( 'code' in data['error'] and data['error']['code'] in ignore_error_codes )Explanation:
The try/except approach:Reduces code complexity and nesting
Improves readability by focusing on the "happy path" logic
Follows Python's "easier to ask forgiveness than permission" (EAFP) idiomSeverity: Important (Not a Nitpick)
This pattern significantly improves code maintainability and readability, especially as dictionary access patterns become more complex.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
clabe/version.pyclabe/banks.py
`**/*.py`: Context Agave is our internal Python library for processing SQS messa...
**/*.py: Context
Agave is our internal Python library for processing SQS messages. The @task decorator can automatically convert JSON to Pydantic models.Rule
Always annotate @task parameters with Pydantic models instead of manually converting dictionaries.Correct Pattern
from pydantic import BaseModel from agave.tasks.sqs_tasks import task class User(BaseModel): name: str age: int @task(queue_url=QUEUE_URL, region_name='us-east-1') async def task_validator(message: User) -> None: # The message is already a User instance - no conversion needed print(message.name) # Direct attribute accessIncorrect Pattern
from pydantic import BaseModel from agave.tasks.sqs_tasks import task class User(BaseModel): name: str age: int @task(queue_url=QUEUE_URL, region_name='us-east-1') async def task_validator(message_data: dict) -> None: # or unannotated parameter # Unnecessary conversion message = User(**message_data) print(message.name)Explanation
The Agave @task decorator automatically:
- Reads JSON messages from SQS queues
- Converts them to Pydantic model instances when the handler parameter is annotated
- Performs validation based on the Pydantic model
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
clabe/version.pyclabe/banks.py
`**/*.py`: ## MANDATORY: Use built-in Pydantic validators
Description
Avoid...
**/*.py: ## MANDATORY: Use built-in Pydantic validatorsDescription
Avoid creating custom validators that duplicate functionality already provided by Pydantic's built-in validators, pydantic_extra_types package, or third-party Pydantic validator libraries. This improves code maintainability and reduces unnecessary unit tests.
Bad Practice
from pydantic import BaseModel, field_validator class MyValidator(BaseModel): location: str @field_validator('location') def validate_location(cls, value: str) -> str: values = value.split(',') if len(values) != 3: raise ValueError('Must provide exactly 3 values for location') # Custom validation logic that duplicates functionality return valueGood Practice
from pydantic import BaseModel from pydantic_extra_types.coordinate import Coordinate class MyValidator(BaseModel): location: CoordinateUnit Test Guidelines
Do not write unit tests specifically for validating the behavior of Pydantic's built-in validators. These are already well-tested by the Pydantic library itself.
Tests to Remove
def test_invalid_location(): pytest.raises(ValidationError): MyValidator(location='foo,bar')Rule Enforcement
This is a mandatory rule, not a refactoring suggestion. Changes must be implemented when:
- A custom validator replicates functionality already available in Pydantic's ecosystem
- There is a suitable built-in, pydantic_extra_types, or third-party Pydantic validator available
Actions required:
- Replace custom validators with appropriate existing validators
- Remove unnecessary unit tests that only validate built-in Pydantic validation behavior
- Block PRs that introduce new custom validators when alternatives exist
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
clabe/version.pyclabe/banks.py
🪛 LanguageTool
README.md
[uncategorized] ~125-~125: En la mayoría de los casos la conjunción ‘pero’ va precedida de una coma.
Context: ...*: Instituciones que aparecen en Banxico pero no están en el paquete - **Bajas de par...
(COMMA_PERO)
[uncategorized] ~125-~125: Probablemente hay un error. Considere aplicar la sugerencia.
Context: ...n en Banxico pero no están en el paquete - Bajas de participantes: Instituciones ...
(AI_ES_GGEC_REPLACEMENT_OTHER)
[uncategorized] ~126-~126: Probablemente hay un error. Considere aplicar la sugerencia.
Context: ... aparecen en la lista oficial de Banxico - Cambios de nombre: Participantes que m...
(AI_ES_GGEC_REPLACEMENT_OTHER)
[uncategorized] ~127-~127: En la mayoría de los casos la conjunción ‘pero’ va precedida de una coma.
Context: ...ticipantes que mantienen el mismo código pero han cambiado su nombre Para ejecutar l...
(COMMA_PERO)
[uncategorized] ~138-~138: Probablemente hay un error. Considere aplicar la sugerencia.
Context: ... reporte con las diferencias encontradas: - ADDITIONS: Nuevos participantes ...
(AI_ES_GGEC_REPLACEMENT_PUNCTUATION)
[uncategorized] ~141-~141: Probablemente hay un error. Considere aplicar la sugerencia.
Context: ...deben eliminarse por ya no estar activos - CHANGES: Cambios de nombre que requier...
(AI_ES_GGEC_REPLACEMENT_OTHER)
🔇 Additional comments (2)
clabe/version.py (1)
1-1: Version bump looks correct
Nothing else changed in this file; the new string'2.1.4'matches the PR scope.clabe/banks.py (1)
85-90: Sync key map vs. name change
The key'715'→'90715'inBANKSis unchanged, while only the display name was updated. That’s expected, but double-check any downstream code that might have cached the old string"Cartera Digital".
actualiza banks.py
Summary by CodeRabbit