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

Fix creating schema from model using ConstrainedStr with regex str as dict key #5223

Merged

Conversation

matejetz
Copy link

@matejetz matejetz commented Mar 19, 2023

Change Summary

Currently, schema creation fails if a model uses a ConstrainedStr model as keys in a dict:

class CustomStr(ConstrainedStr):
    regex = '^text$'

class Model(BaseModel):
    a: Dict[CustomStr, Any]

json_schema = Model.schema()
# AttributeError: 'str' object has no attribute 'pattern'

ConstrainedStr regex field is typed as Optional[Union[str, Pattern[str]], when accessing pattern generating the schema for patternProperty, regex.pattern is accessed.

This PR contains the following changes:

  • Use ConstrainedStr._get_pattern function in schema.py when accessing the pattern
  • Add a test case for the described scenario

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

Selected Reviewer: @samuelcolvin

@matejetz matejetz changed the title Fix custom constr class schema Fix creating schema from model using ConstrainedStr with regex as dict key Mar 19, 2023
@matejetz
Copy link
Author

please review :)

@matejetz matejetz changed the title Fix creating schema from model using ConstrainedStr with regex as dict key Fix creating schema from model using ConstrainedStr with regex str as dict key Mar 19, 2023
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.

We're trying to absolutely minimise the footprint in changes to v1, hence my suggestions.

Please update.

pydantic/types.py Outdated Show resolved Hide resolved
tests/test_schema.py Outdated Show resolved Hide resolved
@matejetz
Copy link
Author

matejetz commented Mar 19, 2023

updated towards minimal footprint, please review

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.

otherwise LGTM.

@dmontagu please review and merge if you're happy.

changes/5223-matejetz.md Outdated Show resolved Hide resolved
@dmontagu
Copy link
Contributor

dmontagu commented Mar 22, 2023

LGTM. I don't like using the "private" ConstrainedStr._get_pattern in a separate module, but I think it's better than duplicating the (admittedly fairly simple) logic (since if it changed it would be easy to miss that it needed an update here), and I don't think it makes sense to make the method public.

If necessary, we can be more thoughtful about the method naming, etc., in v2 see note below, nothing should need changing in v2

@dmontagu dmontagu merged commit ed3e267 into pydantic:1.10.X-fixes Mar 22, 2023
@dmontagu
Copy link
Contributor

Yep, it looks like the code in v2 has been refactored so this particular failure mode can't even happen. So I don't think there's anything to do in terms of implementing a similar fix in v2.

renovate bot added a commit to allenporter/flux-local that referenced this pull request Mar 24, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pydantic](https://togithub.com/pydantic/pydantic) | `==1.10.6` ->
`==1.10.7` |
[![age](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/compatibility-slim/1.10.6)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/confidence-slim/1.10.6)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>pydantic/pydantic</summary>

###
[`v1.10.7`](https://togithub.com/pydantic/pydantic/releases/tag/v1.10.7):
2023-03-22

[Compare
Source](https://togithub.com/pydantic/pydantic/compare/v1.10.6...v1.10.7)

#### What's Changed

- Fix creating schema from model using `ConstrainedStr` with `regex` as
dict key,
[#&#8203;5223](https://togithub.com/pydantic/pydantic/issues/5223) by
[@&#8203;matejetz](https://togithub.com/matejetz)
- Address bug in mypy plugin caused by explicit_package_bases=True,
[#&#8203;5191](https://togithub.com/pydantic/pydantic/issues/5191) by
[@&#8203;dmontagu](https://togithub.com/dmontagu)
- Add implicit defaults in the mypy plugin for Field with no default
argument,
[#&#8203;5190](https://togithub.com/pydantic/pydantic/issues/5190) by
[@&#8203;dmontagu](https://togithub.com/dmontagu)
- Fix schema generated for Enum values used as Literals in discriminated
unions,
[#&#8203;5188](https://togithub.com/pydantic/pydantic/issues/5188) by
[@&#8203;javibookline](https://togithub.com/javibookline)
- Fix mypy failures caused by the pydantic mypy plugin when users define
`from_orm` in their own classes,
[#&#8203;5187](https://togithub.com/pydantic/pydantic/issues/5187) by
[@&#8203;dmontagu](https://togithub.com/dmontagu)
- Fix `InitVar` usage with pydantic dataclasses, mypy version `1.1.1`
and the custom mypy plugin,
[#&#8203;5162](https://togithub.com/pydantic/pydantic/issues/5162) by
[@&#8203;cdce8p](https://togithub.com/cdce8p)

#### New Contributors

- [@&#8203;javibookline](https://togithub.com/javibookline) made their
first contribution in
[pydantic/pydantic#5188
- [@&#8203;matejetz](https://togithub.com/matejetz) made their first
contribution in
[pydantic/pydantic#5223

**Full Changelog**:
pydantic/pydantic@v1.10.6...v1.10.7

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzUuMTcuMSJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to allenporter/pyrainbird that referenced this pull request Mar 24, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pydantic](https://togithub.com/pydantic/pydantic) | `==1.10.6` ->
`==1.10.7` |
[![age](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/compatibility-slim/1.10.6)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/pydantic/1.10.7/confidence-slim/1.10.6)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>pydantic/pydantic</summary>

###
[`v1.10.7`](https://togithub.com/pydantic/pydantic/releases/tag/v1.10.7):
2023-03-22

[Compare
Source](https://togithub.com/pydantic/pydantic/compare/v1.10.6...v1.10.7)

#### What's Changed

- Fix creating schema from model using `ConstrainedStr` with `regex` as
dict key,
[#&#8203;5223](https://togithub.com/pydantic/pydantic/issues/5223) by
[@&#8203;matejetz](https://togithub.com/matejetz)
- Address bug in mypy plugin caused by explicit_package_bases=True,
[#&#8203;5191](https://togithub.com/pydantic/pydantic/issues/5191) by
[@&#8203;dmontagu](https://togithub.com/dmontagu)
- Add implicit defaults in the mypy plugin for Field with no default
argument,
[#&#8203;5190](https://togithub.com/pydantic/pydantic/issues/5190) by
[@&#8203;dmontagu](https://togithub.com/dmontagu)
- Fix schema generated for Enum values used as Literals in discriminated
unions,
[#&#8203;5188](https://togithub.com/pydantic/pydantic/issues/5188) by
[@&#8203;javibookline](https://togithub.com/javibookline)
- Fix mypy failures caused by the pydantic mypy plugin when users define
`from_orm` in their own classes,
[#&#8203;5187](https://togithub.com/pydantic/pydantic/issues/5187) by
[@&#8203;dmontagu](https://togithub.com/dmontagu)
- Fix `InitVar` usage with pydantic dataclasses, mypy version `1.1.1`
and the custom mypy plugin,
[#&#8203;5162](https://togithub.com/pydantic/pydantic/issues/5162) by
[@&#8203;cdce8p](https://togithub.com/cdce8p)

#### New Contributors

- [@&#8203;javibookline](https://togithub.com/javibookline) made their
first contribution in
[pydantic/pydantic#5188
- [@&#8203;matejetz](https://togithub.com/matejetz) made their first
contribution in
[pydantic/pydantic#5223

**Full Changelog**:
pydantic/pydantic@v1.10.6...v1.10.7

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/allenporter/pyrainbird).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xNy4xIiwidXBkYXRlZEluVmVyIjoiMzUuMTcuMSJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

3 participants