Skip to content

Conversation

patrick91
Copy link
Contributor

@patrick91 patrick91 commented Feb 16, 2022

Change Summary

This PR updates the MyPy plugin to not override the __init__ method if there's already one in the model.

Here's an example of the current issue:

https://replit.com/@patrick91/AngryThoseMath#main.py

running mypy main.py returns the following:

main.py:13: error: Too many positional arguments for "Person"
main.py:13: error: Missing named argument "name" for "Person"
main.py:13: error: Missing named argument "birth_year" for "Person"
main.py:15: note: Revealed type is "def (*, id: Any, name: Any, birth_year: Any, **kwargs: Any) -> main.Person"
main.py:16: note: Revealed type is "def (__pydantic_self__: main.Person, *, id: Any, name: Any, birth_year: Any, **kwargs: Any)"

while the code works since we have a custom constructor:

from pydantic import BaseModel


class Person(BaseModel):
    id: int
    name: str
    birth_year: int

    def __init__(self, id: int) -> None:
        super().__init__(id=id, name="Patrick", birth_year=1991)


print(Person(1))

reveal_type(Person)
reveal_type(Person.__init__)

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

@patrick91
Copy link
Contributor Author

coverage fails because we don't hit these lines anymore: https://smokeshow.helpmanual.io/5k4z1l3e0n1q425g2r40/d_2705a9c05af28909_mypy_py.html#t644

I'm not sure if they are needed anymore

@patrick91
Copy link
Contributor Author

please review

@samuelcolvin
Copy link
Member

this is great, thank you so much.

@samuelcolvin
Copy link
Member

Looks like the coverage has dropped, hence this not merging, see here.

Looks like the code which is no-longer covered can be removed, but if you're not sure, maybe safer to add no cover.

Please update.

@github-actions github-actions bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels Aug 9, 2022
@github-actions github-actions bot assigned patrick91 and unassigned samuelcolvin and PrettyWood Aug 9, 2022
auto-merge was automatically disabled August 9, 2022 15:49

Head branch was pushed to by a user without write access

@patrick91
Copy link
Contributor Author

done!

please review

@github-actions github-actions bot added ready for review and removed awaiting author revision awaiting changes from the PR author labels Aug 9, 2022
@samuelcolvin samuelcolvin merged commit f2c3a49 into pydantic:master Aug 10, 2022
@patrick91 patrick91 deleted the mypy/constructor branch August 10, 2022 10:12
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.

4 participants