Skip to content

Conversation

K-dash
Copy link
Contributor

@K-dash K-dash commented Jun 6, 2024

Change Summary

I have added the following test cases to test_pipeline.py:

  • test_ge_le_gt_lt
    • Previously there were only ge and le, so I added test codes for gt and lt.
  • test_parse_multipleOf
    • Previously there were only cases for int, so I added test codes for float and decimal.
  • test_len_constraints
    • Newly added.
  • test_interval_constraints
    • Newly added.
      • I'm not confident if this test case is correctly constructed, so I would like you to review it.
  • test_parse_tz
    • Added because there was no pattern for ValueError.

Pre-PR Coverage of pipeline.py: 83.30%
Current Coverage of pipeline.py w/ this PR: 93.01%

Related issue number

fix #9524

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @sydney-runkle

@K-dash
Copy link
Contributor Author

K-dash commented Jun 6, 2024

please review

Copy link

codspeed-hq bot commented Jun 6, 2024

CodSpeed Performance Report

Merging #9595 will not alter performance

Comparing K-dash:add-experimental-pipline-test (b687734) with main (6055019)

Summary

✅ 13 untouched benchmarks

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Amazing, thanks so much @K-dash!

What did you think about this API? We're looking for feedback on this new experimental feature 🚀 !

@sydney-runkle sydney-runkle merged commit 2c35753 into pydantic:main Jun 6, 2024
@K-dash
Copy link
Contributor Author

K-dash commented Jun 7, 2024

@sydney-runkle
Thank you for merging the PR!

What did you think about this API? We're looking for feedback on this new experimental feature 🚀 !

I genuinely think this is a great feature.

In the past, when defining complex constraints and transformations for model fields in Pydantic, we had to use separate Field Validators and Model Validators. However, with the introduction of this pipeline API, I believe it has enhanced type safety and improved readability with more concise syntax.

If there are any other benefits or effective ways to use this pipeline API, I would love to learn about them. :)

I understand this is currently an experimental feature, but I look forward to incorporating it once it is officially released. Thank you for implementing such a useful feature!

@K-dash K-dash deleted the add-experimental-pipline-test branch June 7, 2024 06:02
@sydney-runkle
Copy link
Contributor

@K-dash,

Wonderful, thanks so much for your feedback! We're super excited to get your feedback. @adriangb did all of the initial work on this awesome feature, and even wrote a PEP to help support this! https://peps.python.org/pep-0746/

I think the main benefits are:

  1. Type safety
  2. Readability
  3. Ease of reuse
  4. Customizing constraint orders (like for string constraints, which isn't currently easily doable without this feature)

I'm sure there are even more things, but these were the main motivating factors!

@K-dash
Copy link
Contributor Author

K-dash commented Jun 8, 2024

@sydney-runkle
Thank you for your response.
I have looked at PEP 746 created by Adrian.
I see, in order to validate the consistency between the type annotations specified in Annotated and the associated metadata, the metadata needs to implement the __supports_type__ protocol.
I hadn't understood that part.

  1. Ease of reuse
  2. Customizing constraint orders (like for string constraints, which isn't currently easily doable without this feature)

Indeed, I also think the above two benefits are significant. Thank you.
Through the implementation of these test codes, I have learned a lot not only about the pipeline API but also about type annotations. I am very grateful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase tests coverage for new pipeline API
2 participants