CodSpeed Performance Report

Merging #11016 will not alter performance

_{Comparing KCui0327:decimal_constraint (c159bea) with main (32f405b)}

Summary

✅ 45 untouched benchmarks

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
pydantic
json_schema.py					712-720
Project Total

This report was generated by python-coverage-comment-action

please review (thanks!)

Thanks for the PR. A couple remarks on the pattern implementation:

* Can't we also set a pattern if only `decimal_places` is set?

* I'm wondering if we should special case leading zeros differently. What we could do is:
  
  * in validation, allow arbitrary leading zeros to be specified. Pydantic will not error during validation.
  * in serialization, only allow a single 0 or `[1-9]` followed by the constrained number of integers.

* Taking into account what was said above, let's define the pattern as a private constant in this module, and explain it with comments (and then compile with the `re.VERBOSE` flag), e.g:
  ```python
   _DECIMAL_PATTERN = r"""
   ^-?      # Possible minus sign
   ...
   """
  ```

Hi @Viicos, thank you for your feedback! I have address your feedback in the latest commits, please let me know if there's any other changes I should add.

Hey @KCui0327,

Are you still interested in working on this? Thanks for your contributions thus far!

Hey @KCui0327,

Are you still interested in working on this? Thanks for your contributions thus far!

Hi @sydney-runkle ,

Apologies for the late response. I will not be able to actively contribute to this PR for the next few months so I want to open it to anyone who wants to take this PR further and address the comments from Viccos.

at pydantify we are looking forward to have this merged as well. Thanks for all the work on this PR

Hi @sydney-runkle and @Viicos

I can have a look at finishing this off. I should be able to submit an updated PR later this week.

I was reviewing the current PR and noticed the use of a verbose regex pattern makes its way through to the json_schema() output, and it doesn't look like a re.Pattern object provides an easy way to extract the regex without the comments and extra white space.

{'anyOf': [{'maximum': 10.0, 'minimum': 0.0, 'type': 'number'}, {'pattern': '\n ^\n -? # Minus sign (optional)\n \\d+ # One or more digits for the integer part (allows leading zeros)\n ( # Optional decimal group\n \\. # Decimal point\n \\d{0,2} # Up to 2 decimal digits\n )?\n $\n', 'type': 'string'}]}

Thanks for looking into it.

My bad, I thought compiling the verbose pattern and using the pattern attribute would give us the final pattern with comments and extra spaces stripped.

Instead, lets use a simple multi-line string and add comments this way:

_DECIMAL_JSON_SERIALIZATION_PATTERN = (
    r"^-?"                                  # Minus sign (optional)
    r"(?:0|[1-9]\d{{0,{integer_places}}})"  # Integer part: single zero OR single non-zero digit...
)

Looks like this will be finished up by #11420, thanks for the initial work on this PR!