Skip to content

Conversation

dmontagu
Copy link
Contributor

@dmontagu dmontagu commented Jun 21, 2019

Fix #609 (default arguments for GenericModel)

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • HISTORY.rst has been updated

@codecov
Copy link

codecov bot commented Jun 21, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@3ee54ed). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             master   #610   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?     15           
  Lines             ?   2582           
  Branches          ?    510           
=======================================
  Hits              ?   2582           
  Misses            ?      0           
  Partials          ?      0

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.

maybe we should add a test for the case of a default as per the issue

def test_default_arguments():
    T = TypeVar('T')

    class Result(GenericModel, Generic[T]):
        data: T
        other: bool = True
    result = Result[int](data=1)

?

@dmontagu
Copy link
Contributor Author

maybe we should add a test for the case of a default as per the issue

def test_default_arguments():
    T = TypeVar('T')

    class Result(GenericModel, Generic[T]):
        data: T
        other: bool = True
    result = Result[int](data=1)

?

Sorry my comments were confusing -- when I said I added tests not directly related, I meant in addition to one that was.

The following test was already committed:

@skip_36
def test_default_arguments():
    T = TypeVar('T')

    class Result(GenericModel, Generic[T]):
        data: T
        other: bool = True

    result = Result[int](data=1)
    assert result.other is True

Let me know if that's not enough

@samuelcolvin
Copy link
Member

sound more double quotes I'm afraid :-). Promise I'll do linting for this soon.

Also, what about

class Result(GenericModel, Generic[T]):
    data: T = 4

Does that work fine? Maybe worth adding a test?

@dmontagu
Copy link
Contributor Author

dmontagu commented Jun 21, 2019

sound more double quotes I'm afraid :-). Promise I'll do linting for this soon.

+1 here's an option

Also, what about

class Result(GenericModel, Generic[T]):
    data: T = 4

Does that work fine? Maybe worth adding a test?

I'm not sure if I should be reading anything special into this (e.g., raise errors if T isn't numeric at model creation time?), but it appears to work correctly (or at least, the same as it would if inheriting from BaseModel with T replaced). I added the test.

David Montague and others added 7 commits June 23, 2019 00:50
@samuelcolvin samuelcolvin merged commit 3f754c8 into pydantic:master Jun 25, 2019
alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023
…es (pydantic#610)

Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default values ignored for GenericModel
2 participants