Skip to content

Try to evaluate forward refs after model created#2588

Merged
samuelcolvin merged 6 commits intopydantic:masterfrom
uriyyo:feature/auto-forward-refs
Dec 5, 2021
Merged

Try to evaluate forward refs after model created#2588
samuelcolvin merged 6 commits intopydantic:masterfrom
uriyyo:feature/auto-forward-refs

Conversation

@uriyyo
Copy link
Copy Markdown
Contributor

@uriyyo uriyyo commented Mar 28, 2021

Change Summary

Try to evaluate forward refs after model created.

So it will not be required to use update_forward_refs in the case of self-referencing model:

from __future__ import annotations

from pydantic import BaseModel
from typing import Optional


class Foo(BaseModel):
    foo: Optional[Foo]


assert Foo.__fields__['foo'].type_ is Foo

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)

PrettyWood
PrettyWood reviewed Apr 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

Can you please rebase/merge with master to trigger the new CI?

Comment thread pydantic/main.py Outdated
@PrettyWood PrettyWood added the awaiting author revision awaiting changes from the PR author label Apr 5, 2021
@uriyyo uriyyo force-pushed the feature/auto-forward-refs branch from fc89c5a to 286b730 Compare April 5, 2021 18:06
@PrettyWood PrettyWood added ready for review and removed awaiting author revision awaiting changes from the PR author labels Apr 5, 2021
@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented May 9, 2021

you have conflicts and the wrong change.

Please update.

@github-actions github-actions Bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels May 9, 2021
samuelcolvin
samuelcolvin reviewed May 9, 2021
View reviewed changes
Copy link
Copy Markdown
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.

looking good, but this also needs a note in the docs.

Comment thread pydantic/main.py Outdated
Comment thread pydantic/main.py Outdated
@uriyyo uriyyo force-pushed the feature/auto-forward-refs branch from 286b730 to cb5ee00 Compare May 9, 2021 15:20
@uriyyo
Copy link
Copy Markdown
Contributor Author

uriyyo commented May 9, 2021

@samuelcolvin I have updated PR. Could you please review it again?

@uriyyo uriyyo requested a review from samuelcolvin May 9, 2021 15:23
@github-actions github-actions Bot added ready for review and removed awaiting author revision awaiting changes from the PR author labels May 9, 2021
@github-actions github-actions Bot assigned PrettyWood and samuelcolvin and unassigned uriyyo May 9, 2021
PrettyWood
PrettyWood reviewed Sep 4, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

Cool feature! Nice to be able to do everything automatically

Comment thread changes/2588-uriyyo.md Outdated
Comment thread docs/usage/postponed_annotations.md Outdated
Comment thread pydantic/main.py
cls = super().__new__(mcs, name, bases, new_namespace, **kwargs)
# set __signature__ attr only for model class, but not for its instances
cls.__signature__ = ClassAttribute('__signature__', generate_model_signature(cls.__init__, fields, config))
cls.__try_update_forward_refs__()
Copy link
Copy Markdown
Contributor

@PrettyWood PrettyWood Sep 4, 2021

Choose a reason for hiding this comment

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

What is the impact on perf for models without forward refs? Do we have a way to know whether or not we should try to update?

Copy link
Copy Markdown
Contributor Author

@uriyyo uriyyo Sep 4, 2021

Choose a reason for hiding this comment

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

I wrote a simple benchmark:

import timeit
from pydantic import BaseModel


def callback():
    class Order(BaseModel):
        a: int
        b: float
        c: str


t = timeit.timeit(
    callback,
    number=10_000,
    globals={"BaseModel": BaseModel},
)

print(f'{t:.5f}')

Results:

  • With __try_update_forward_refs__ - 3.17182
  • Without __try_update_forward_refs__ - 3.15772

@PrettyWood
Copy link
Copy Markdown
Contributor

PrettyWood commented Sep 4, 2021

Hi @uriyyo
Can you please rebase/merge and update? 🙏

@PrettyWood PrettyWood assigned uriyyo and unassigned samuelcolvin and PrettyWood Sep 4, 2021
@PrettyWood PrettyWood added awaiting author revision awaiting changes from the PR author and removed ready for review labels Sep 4, 2021
uriyyo and others added 3 commits September 4, 2021 21:20
@uriyyo @PrettyWood
Update changes/2588-uriyyo.md
6b8f0d1 
Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>
@uriyyo @PrettyWood
Update docs/usage/postponed_annotations.md
31ba3e1 
Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>
@uriyyo
Merge remote-tracking branch 'origin/master' into feature/auto-forwar…
06583fb 
…d-refs

# Conflicts:
#	pydantic/main.py
PrettyWood
PrettyWood reviewed Sep 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

Yes I run the benchmark seems ok performance-wise

This branch

▶ BENCHMARK_REPEATS=20 make benchmark-pydantic
python benchmarks/run.py pydantic-only
testing pydantic, 20 times each
    pydantic ( 1/20) time=0.622s, success=48.65%
    pydantic ( 2/20) time=0.617s, success=48.65%
    pydantic ( 3/20) time=0.598s, success=48.65%
    pydantic ( 4/20) time=0.596s, success=48.65%
    pydantic ( 5/20) time=0.597s, success=48.65%
    pydantic ( 6/20) time=0.606s, success=48.65%
    pydantic ( 7/20) time=0.595s, success=48.65%
    pydantic ( 8/20) time=0.598s, success=48.65%
    pydantic ( 9/20) time=0.591s, success=48.65%
    pydantic (10/20) time=0.597s, success=48.65%
    pydantic (11/20) time=0.595s, success=48.65%
    pydantic (12/20) time=0.605s, success=48.65%
    pydantic (13/20) time=0.603s, success=48.65%
    pydantic (14/20) time=0.601s, success=48.65%
    pydantic (15/20) time=0.611s, success=48.65%
    pydantic (16/20) time=0.603s, success=48.65%
    pydantic (17/20) time=0.600s, success=48.65%
    pydantic (18/20) time=0.596s, success=48.65%
    pydantic (19/20) time=0.592s, success=48.65%
    pydantic (20/20) time=0.605s, success=48.65%
    pydantic best=0.591s, avg=0.601s, stdev=0.008s

    pydantic best=98.503μs/iter avg=100.240μs/iter stdev=1.335μs/iter version=1.8.2

1.8

▶ BENCHMARK_REPEATS=20 make benchmark-pydantic
python benchmarks/run.py pydantic-only
testing pydantic, 20 times each
    pydantic ( 1/20) time=0.608s, success=48.65%
    pydantic ( 2/20) time=0.588s, success=48.65%
    pydantic ( 3/20) time=0.616s, success=48.65%
    pydantic ( 4/20) time=0.632s, success=48.65%
    pydantic ( 5/20) time=0.599s, success=48.65%
    pydantic ( 6/20) time=0.588s, success=48.65%
    pydantic ( 7/20) time=0.591s, success=48.65%
    pydantic ( 8/20) time=0.595s, success=48.65%
    pydantic ( 9/20) time=0.589s, success=48.65%
    pydantic (10/20) time=0.612s, success=48.65%
    pydantic (11/20) time=0.599s, success=48.65%
    pydantic (12/20) time=0.594s, success=48.65%
    pydantic (13/20) time=0.623s, success=48.65%
    pydantic (14/20) time=0.591s, success=48.65%
    pydantic (15/20) time=0.592s, success=48.65%
    pydantic (16/20) time=0.595s, success=48.65%
    pydantic (17/20) time=0.595s, success=48.65%
    pydantic (18/20) time=0.600s, success=48.65%
    pydantic (19/20) time=0.625s, success=48.65%
    pydantic (20/20) time=0.587s, success=48.65%
    pydantic best=0.587s, avg=0.601s, stdev=0.014s

    pydantic best=97.825μs/iter avg=100.158μs/iter stdev=2.254μs/iter version=1.8

PrettyWood
PrettyWood approved these changes Sep 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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!

@PrettyWood PrettyWood added ready for review and removed awaiting author revision awaiting changes from the PR author labels Sep 5, 2021
@samuelcolvin samuelcolvin merged commit 415eb54 into pydantic:master Dec 5, 2021
@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented Dec 5, 2021

this is great, thank you so much.

PrettyWood added a commit to PrettyWood/pydantic that referenced this pull request Dec 6, 2021
@uriyyo @PrettyWood
Try to evaluate forward refs after model created (pydantic#2588)
cf59e12 
* Try to evaluate forward refs after model created

* Upadate docs and remove code duplication

* Update changes/2588-uriyyo.md

Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>

* Update docs/usage/postponed_annotations.md

Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>

* Remove unused import

Co-authored-by: Eric Jolibois <em.jolibois@gmail.com>
@PrettyWood PrettyWood mentioned this pull request Jan 20, 2022
Closed
3 tasks
@followben followben mentioned this pull request Feb 11, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelcolvin samuelcolvin Awaiting requested review from samuelcolvin

1 more reviewer

@PrettyWood PrettyWood PrettyWood approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@uriyyo uriyyo

Labels

ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@uriyyo @samuelcolvin @PrettyWood