Skip to content

fix(fields): handle properly default value for type Callable#2094

Merged
samuelcolvin merged 8 commits intopydantic:masterfrom
PrettyWood:fix/callable-default-value
Feb 11, 2021
Merged

fix(fields): handle properly default value for type Callable#2094
samuelcolvin merged 8 commits intopydantic:masterfrom
PrettyWood:fix/callable-default-value

Conversation

@PrettyWood
Copy link
Copy Markdown
Contributor

@PrettyWood PrettyWood commented Nov 5, 2020 •
edited
Loading

Change Summary

from typing import Callable

from pydantic import BaseModel


def pika_default() -> str:
    return 'Pika is the best'

class PikaModel(BaseModel):
    pika: Callable[[], str] = pika_default


print(PikaModel.__fields__)

BEFORE

{}

AFTER

{'pika': ModelField(name='pika', type=Callable[[], str], required=False, default=<function pika_default at 0x10a6541f0>)}

Related issue number

closes #1596
closes #2086

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)

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 5, 2020 •
edited
Loading

Codecov Report

Merging #2094 (904c2f5) into master (13a5c7d) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2094   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         4199      4199           
  Branches       854       854           
=========================================
  Hits          4199      4199
Impacted Files Coverage Δ
pydantic/schema.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13a5c7d...904c2f5. Read the comment docs.

samuelcolvin
samuelcolvin reviewed Nov 29, 2020
View reviewed changes
Comment thread pydantic/main.py Outdated


UNTOUCHED_TYPES = FunctionType, property, type, classmethod, staticmethod
FIELD_DEFAULT_VALUE_UNTOUCHED_TYPES = property, type, classmethod, staticmethod
Copy link
Copy Markdown
Member

@samuelcolvin samuelcolvin Nov 29, 2020

Choose a reason for hiding this comment

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

can we make this name shorter. I think we need to describe what's going in here in a comment.

Copy link
Copy Markdown
Contributor Author

@PrettyWood PrettyWood Nov 29, 2020

Choose a reason for hiding this comment

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

I added a comment and renamed it to ANNOTATED_FIELD_UNTOUCHED_TYPES. Still a bit long but I couldn't find an explicit name shorter than that.

Comment thread tests/test_main.py Outdated
Comment thread tests/test_main.py Outdated
@PrettyWood PrettyWood force-pushed the fix/callable-default-value branch from 1e75ab1 to cfba22c Compare November 29, 2020 22:36
samuelcolvin
samuelcolvin reviewed Dec 31, 2020
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.

otherwise LGTM

Comment thread tests/test_dataclasses.py Outdated
def non_default_callable(x):
return foo(x, 'nondefault')

for cls in (HasCallablesModel, HasCallablesDC):
Copy link
Copy Markdown
Member

@samuelcolvin samuelcolvin Dec 31, 2020

Choose a reason for hiding this comment

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

it's really confusing if something fails to work out which object failed.

Better just to split the test and duplicate code, or if there's a lot of duplication, use fixtures.

Copy link
Copy Markdown
Contributor Author

@PrettyWood PrettyWood Jan 2, 2021

Choose a reason for hiding this comment

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

I copied the test from the issue but I agree it was a bit confusing. I rewrote the test. Hope it's easier to read.
Also added with an annotated callable like Callable[[int, float, str], bool].
Ready for another review :)

Comment thread tests/test_dataclasses.py Outdated
default_callable_factory: Callable = dataclasses.field(default=lambda x: foo(x, 'factory'))

class HasCallablesModel(BaseModel):
non_default_callable: Callable
Copy link
Copy Markdown
Member

@samuelcolvin samuelcolvin Dec 31, 2020

Choose a reason for hiding this comment

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

what about typed callables like Callable[[int, float, str], bool]?

Surely worth adding tests for them too?

@samuelcolvin samuelcolvin mentioned this pull request Jan 1, 2021
Merged
4 tasks
@PrettyWood
Copy link
Copy Markdown
Contributor Author

PrettyWood commented Jan 3, 2021

Added a small commit for schema generation in the batch since it's related (callable fields with default value) even though it's not the same root cause 😉

@PrettyWood PrettyWood mentioned this pull request Feb 2, 2021
Closed
3 tasks
@samuelcolvin samuelcolvin merged commit d0baf0f into pydantic:master Feb 11, 2021
@PrettyWood PrettyWood deleted the fix/callable-default-value branch February 11, 2021 12:09
@aminalaee aminalaee mentioned this pull request Jun 11, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelcolvin samuelcolvin samuelcolvin left review comments

Assignees

No one assigned

Labels

ready for review

Projects

None yet

Milestone

No milestone

2 participants

@PrettyWood @samuelcolvin