Navigation Menu

Skip to content
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

Add support for re.Pattern #4366

Merged
merged 1 commit into from Aug 12, 2022
Merged

Conversation

hramezani
Copy link
Member

@hramezani hramezani commented Aug 11, 2022

Change Summary

Add support for re.Pattern

Related issue number

ref #4360
This PR just address the re.pattern

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@hramezani hramezani force-pushed the re_pattern branch 3 times, most recently from 9f1d3f4 to 66d9348 Compare August 11, 2022 22:13
@hramezani
Copy link
Member Author

please review

Copy link
Member

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@Bobronium Bobronium left a comment

Choose a reason for hiding this comment

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

This doesn't include changes to schema generation, mentioned in #4360.

if lenient_issubclass(field_type, type_) or field_type is type_ is Pattern:

So generating schema with re.Pattern most likely gonna blow up.

@hramezani
Copy link
Member Author

This doesn't include changes to schema generation, mentioned in #4360.

if lenient_issubclass(field_type, type_) or field_type is type_ is Pattern:

So generating schema with re.Pattern most likely gonna blow up.

I've checked this as well but it seems we don't need it because the lenient_issubclass(field_type, type_) returns True for re.Pattern

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

Let's add a test for schema, just to be sure.

Also Please remember "fix #xxx" in the PR body, I won't always remember to update it.

please update.

@hramezani
Copy link
Member Author

Let's add a test for schema, just to be sure.

The test for the schema is part of test_pattern

Also Please remember "fix #xxx" in the PR body, I won't always remember to update it.

I didn't add fix because this PR just add re.Pattern and the issue is about adding re.Pattern and type support. I am going to create another PR for type later. I don't want to close the issue after we merge this one.

@samuelcolvin
Copy link
Member

Good point, sorry my mistake.

@hramezani
Copy link
Member Author

please review

@samuelcolvin samuelcolvin merged commit 104748c into pydantic:master Aug 12, 2022
@samuelcolvin
Copy link
Member

thanks so much.

@hramezani hramezani deleted the re_pattern branch August 12, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants