Skip to content

Rework create_model field definitions format #11032

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

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Conversation

Viicos
Copy link
Member

@Viicos Viicos commented Dec 3, 2024

Fixes #11005, fixes #11015, closes #11006.

If the value is a two-tuple, the first element is the type and the second element is the assigned value (either a default or a Field() function). This isn't breaking as this was already allowed before.

We now also allow a single element to be passed, which will be the type. This also isn't breaking because:

  • We previously allowed a single element to be passed if it was an annotated type. We then extracted the first metadata element and enforced it to be a Field() function. With these changes, we now just leave the annotated type as is, so an arbitrary amount of metadata can now be specified.
  • If the type wasn't annotated, we were raising an error.

Private fields are now allowed.

Redundant tests were merged/removed. Documentation was updated.

The __slots__ parameter was removed. It was deprecated (with a wrong warning class, but still) since 1.10, was not doing anything, so I don't expect anyone to be using it.

Change Summary

Related issue number

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

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Dec 3, 2024
Copy link

cloudflare-workers-and-pages bot commented Dec 3, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: a0c253b
Status: ✅  Deploy successful!
Preview URL: https://ba5c77de.pydantic-docs.pages.dev
Branch Preview URL: https://create-model-rework.pydantic-docs.pages.dev

View logs

If the value is a two-tuple, the first element is the type and
the second element is the assigned value (either a default or
a `Field()` function). This isn't breaking as this was already
allowed before.

We now also allow a single element to be passed, which will be
the type. This also isn't breaking because:
- We previously allowed a single element to be passed if it was
  an annotated type. We then extracted the first metadata element
  and enforced it to be a `Field()` function. With these changes,
  we now just leave the annotated type as is, so an arbitrary amount
  of metadata can now be specified.
- If the type wasn't annotated, we were raising an error.

Private fields are now allowed.

Redundant tests were merged/removed. Documentation was updated.

The `__slots__` parameter was removed. It was deprecated (with
a wrong warning class, but still) since 1.10, was not doing anything,
so I don't expect anyone to be using it.
@Viicos Viicos force-pushed the create-model-rework branch from 0c70c39 to 46ef644 Compare December 3, 2024 12:10
Copy link

codspeed-hq bot commented Dec 3, 2024

CodSpeed Performance Report

Merging #11032 will not alter performance

Comparing create-model-rework (a0c253b) with main (c6905b6)

Summary

✅ 46 untouched benchmarks

Copy link
Contributor

github-actions bot commented Dec 3, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  main.py
Project Total  

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

Seems like a common error was to write tests in this file,
because it was named `test_create_model.py`.
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.

Thanks @Viicos, solid improvement here.

Nice work with the docs updates. Also, pretty simple implementation change, which is nice as well.

A few follow up questions, but otherwise, looking great!

@sydney-runkle sydney-runkle added relnotes-change Used for changes to existing functionality which don't have a better categorization. and removed relnotes-fix Used for bugfixes. labels Dec 3, 2024
@sydney-runkle
Copy link
Contributor

I've added the relnotes-change label, as we should highlight the official end of support for __slots__ in the release notes here.

As you mentioned, deprecated long ago, so I'm not too worried about breakages here.

@Viicos Viicos enabled auto-merge (squash) December 3, 2024 17:16
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.

Nice work, excited to have this be more flexible.

@Viicos Viicos merged commit c409bc7 into main Dec 3, 2024
56 checks passed
@Viicos Viicos deleted the create-model-rework branch December 3, 2024 17:23
amritghimire added a commit to iterative/datachain that referenced this pull request Mar 28, 2025
With recent release in pydantic, the pickling is not working as
expected. This restricts the pydantic to 2.10.6 untill we fix the
breaking changes.

Relevant links:
- https://github.com/pydantic/pydantic/releases/tag/v2.11.0
- pydantic/pydantic#11032
- https://iterativeai.slack.com/archives/C04A9RWEZBN/p1743143098907759
amritghimire added a commit to iterative/datachain that referenced this pull request Mar 28, 2025
With recent release in pydantic, the pickling is not working as
expected. This restricts the pydantic to 2.10.6 untill we fix the
breaking changes.

Relevant links:
- https://github.com/pydantic/pydantic/releases/tag/v2.11.0
- pydantic/pydantic#11032
- https://iterativeai.slack.com/archives/C04A9RWEZBN/p1743143098907759
@commonism
Copy link
Contributor

I'm look at this as I suspect this causes multiple regressions 2.10.6 -> 2.11.0 when using create_model

known to me:

  • using model_config as field (easy, using config instead)
  • using a property(…) as field (yet unknown)

as there is multiple issues linked to this change - any chance this gets revisited?

@Viicos
Copy link
Member Author

Viicos commented Apr 11, 2025

as there is multiple issues linked to this change - any chance this gets revisited?

As mentioned in the linked issues, this wasn't documented/expected, in particular for model_config vs __config__. This is a typical https://xkcd.com/1172/ situation, so nothing will change for 2.11. For properties, this is unfortunate but it was clearly documented that the extra keyword arguments are fields.

@Viicos
Copy link
Member Author

Viicos commented Apr 11, 2025

At the very least, we could backport the fix for #11709 in the next patch release.

@commonism
Copy link
Contributor

commonism commented Apr 11, 2025

Let's forget about model_config/config, inconvenient but won't cause any long lasting headaches.

For computed fields and properties, as it works for models created by inheriting from BaseModel, there is nothing wrong in the expectation create_model can do the same. And it did the same for about two years.

As an MRE for property issue:

from pydantic import create_model, ConfigDict


def mkx():
    def get_additionalProperties(x):
        return x.model_extra

    return get_additionalProperties, None, None


A = create_model(
    'A',
    __config__=ConfigDict(extra="allow"),
    aio3_additionalProperties=(None, property(mkx()[0])),
)

a = A(x=1)
assert not isinstance(a.aio3_additionalProperties, property)

A possible fix - make the annotation declaration optional:

pydantic/pydantic/main.py

Lines 1742 to 1746 in 2e7cddb

annotations[f_name] = f_def[0]
fields[f_name] = f_def[1]
else:
annotations[f_name] = f_def

            if f_def[0]:
                annotations[f_name] = f_def[0]
            fields[f_name] = f_def[1]
        else:
            annotations[f_name] = f_def

commonism added a commit to commonism/aiopenapi3 that referenced this pull request Apr 12, 2025
commonism added a commit to commonism/aiopenapi3 that referenced this pull request Apr 12, 2025
@Viicos
Copy link
Member Author

Viicos commented Apr 13, 2025

there is nothing wrong in the expectation create_model can do the same.

There is, as I mention and this can be seen in the 2.10 documentation:

image

We haven't decided on a solution yet, but it will probably not be using the **kwargs. #11709 is the issue tracking this, and the implementation will be backported to 2.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes-change Used for changes to existing functionality which don't have a better categorization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow field names starting with underscore in create_model if they are class vars support multiple Field in Annotated for create_model()
3 participants