Skip to content

Seguir la guia de Pydantic para suprimir una advertencia #182

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.

Sign up for GitHub

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

Open
FanGoH wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Seguir la guia de Pydantic para suprimir una advertencia #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion clabe/validations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ class BankConfigRequest(BaseModel):

bank_name: str = Field(
min_length=1,
strip_whitespace=True,
description="Bank name must have at least 1 character.",
json_schema_extra={
"strip_whitespace": True,
}
)
Comment on lines 77 to 83
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

CRITICAL: Whitespace stripping behavior is broken.

Moving strip_whitespace to json_schema_extra only adds metadata to the JSON schema—it does not preserve the runtime whitespace stripping behavior that existed in Pydantic v1. The field will no longer strip whitespace from bank names during validation.

For Pydantic v2, use StringConstraints to preserve the behavior:

-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, StringConstraints
+from typing import Annotated

 class BankConfigRequest(BaseModel):
     """
     Validates and processes bank configuration requests.

     The class handles validation of bank names and codes, ensuring:
     - Bank names are non-empty strings
     - Banxico codes are exactly 5 digits
     """

-    bank_name: str = Field(
+    bank_name: Annotated[str, StringConstraints(strip_whitespace=True)] = Field(
         min_length=1,
         description="Bank name must have at least 1 character.",
-        json_schema_extra={
-            "strip_whitespace": True,
-        }
     )

This follows the Pydantic v2 migration guide and maintains the actual whitespace stripping functionality. As per coding guidelines.

Verify the current behavior with whitespace:

🏁 Script executed:

#!/bin/bash
# Description: Test if whitespace stripping still works

python3 << 'EOF'
from clabe.validations import BankConfigRequest

# Test with leading/trailing whitespace
try:
    request = BankConfigRequest(
        bank_code_banxico="90716",
        bank_name="  Test Bank  "
    )
    print(f"bank_name value: '{request.bank_name}'")
    print(f"Whitespace stripped: {request.bank_name == 'Test Bank'}")
except Exception as e:
    print(f"Error: {e}")
EOF

Length of output: 59

CRITICAL: Whitespace stripping behavior is broken—verification confirms field retains spaces.

The test confirms that moving strip_whitespace to json_schema_extra does not preserve whitespace stripping. Input " Test Bank " remains unchanged instead of being stripped to "Test Bank". This violates the coding guideline requiring built-in Pydantic validators.

Fix at lines 77–83 using StringConstraints and Annotated:

from typing import Annotated
from pydantic import StringConstraints

bank_name: Annotated[str, StringConstraints(strip_whitespace=True)] = Field(
    min_length=1,
    description="Bank name must have at least 1 character.",
)

This is the correct Pydantic v2 approach and will restore runtime whitespace stripping.

🤖 Prompt for AI Agents 
In clabe/validations.py around lines 77–83, the Field currently uses
json_schema_extra to set strip_whitespace which does not perform runtime
stripping; replace that with Pydantic v2 string constraints by switching the
annotation to use typing.Annotated with
pydantic.StringConstraints(strip_whitespace=True) and remove strip_whitespace
from json_schema_extra so the bank_name field becomes an Annotated[str,
StringConstraints(strip_whitespace=True)] with the existing Field(min_length=1,
description=...) to restore runtime whitespace stripping.

Copy link
Member

@felipao-mx felipao-mx Nov 13, 2025

Choose a reason for hiding this comment

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

hola @FanGoH gracias por tu aporte! Observo algunos detalles, revisé la doc y en efecto esto suprime los warnings pero no corrige el issue principal que es hacer que el validator suprima espacios en blanco. Puedes revisar la sugerencia de @coderabbitai por favor?

Copy link

@coderabbitai coderabbitai bot Nov 13, 2025

Choose a reason for hiding this comment

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

@felipao-mx Thank you for confirming! You're absolutely right - the current approach only suppresses the warning but loses the actual whitespace stripping behavior at runtime.

To summarize for @FanGoH:

  • Current PR: Moves strip_whitespace=True to json_schema_extra → ❌ Breaks functionality (only adds schema metadata)
  • Correct fix: Use Annotated[str, StringConstraints(strip_whitespace=True)] → ✅ Preserves both functionality and Pydantic v2 compatibility

The suggested approach follows the official Pydantic v2 migration guide and will:

  1. ✅ Suppress the deprecation warning
  2. ✅ Maintain the whitespace stripping behavior
  3. ✅ Prepare for Pydantic v3 compatibility

Happy to help verify the fix once implemented! 🚀


bank_code_banxico: str = Field(
Expand Down
7 changes: 7 additions & 0 deletions tests/test_clabe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warnings
import pytest

from clabe import (
Expand Down Expand Up @@ -55,6 +56,12 @@ def test_add_bank_success(abm_code, banxico_code, name):
assert get_bank_name(abm_code) == name


def test_add_bank_no_show_warnings():
with warnings.catch_warnings(record=True) as caught_warnings:
warnings.simplefilter("always")
add_bank("90716", "Test bank")
assert len(caught_warnings) == 0, "Warnings were emitted during add_bank"
Comment on lines +59 to +63
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add test coverage for whitespace stripping behavior.

While this test correctly verifies that no warnings are emitted, it doesn't validate the actual whitespace stripping behavior that was supposedly preserved from the Pydantic migration. Consider testing with whitespace in the bank name (e.g., add_bank("90716", " Test Bank ")) to ensure the behavior is maintained.

🤖 Prompt for AI Agents 
In tests/test_clabe.py around lines 59 to 63, the test only asserts no warnings
but doesn't verify that add_bank strips surrounding whitespace from the bank
name; update the test to call add_bank("90716", "  Test Bank  ") (while still
using warnings.catch_warnings) and assert that the returned/recorded bank name
equals "Test Bank" (or lookup the stored bank entry and assert its name is
stripped), in addition to preserving the existing assertion that no warnings
were emitted.


@pytest.mark.parametrize(
'banxico_code, name',
[
Expand Down
7 changes: 7 additions & 0 deletions tests/test_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warnings
import pytest
from pydantic import BaseModel, ValidationError

Expand All @@ -19,6 +20,12 @@ def test_valid_clabe():
assert cuenta.clabe.bank_code == cuenta.clabe.bank_code_banxico


def test_valid_clabe_shows_no_warnings():
with warnings.catch_warnings(record=True) as caught_warnings:
warnings.simplefilter("always")
Cuenta(clabe=VALID_CLABE)
assert len(caught_warnings) == 0, "Warnings were emitted during CLABE test"

@pytest.mark.parametrize(
'clabe,expected_message',
[
Expand Down