Skip to content
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

Better type hints for model_bakery.recipe #292

Merged
merged 5 commits into from Mar 31, 2022
Merged

Conversation

hoefling
Copy link
Contributor

Motivation

Using generic types in model_bakery.recipe module allows for better hints from IDEs (if supported). Generic types are part of the stdlib's typing since 3.5, so the changes are conform with the range of Python versions currently supported by the library (although note that 3.5 and 3.6 are EOL already).

Demo (Pycharm)

image
image
image

Demo (mypy)

Sample code snippet:

from django.db.models import Model, ForeignKey
from model_bakery.recipe import Recipe, foreign_key, related

class C(Model):
    ...

r = Recipe(C)

reveal_type(r)
reveal_type(r.extend(spam="eggs"))
reveal_type(r.make())
reveal_type(r.make(_quantity=None))
reveal_type(r.make(_quantity=100))
reveal_type(foreign_key(Recipe(C)))
reveal_type(related(r, r))

On current mainline, mypy will emit

test.py:14: note: Revealed type is "model_bakery.recipe.Recipe"
test.py:15: note: Revealed type is "model_bakery.recipe.Recipe"
test.py:16: note: Revealed type is "Union[django.db.models.base.Model, builtins.list[django.db.models.base.Model]]"
test.py:17: note: Revealed type is "Union[django.db.models.base.Model, builtins.list[django.db.models.base.Model]]"
test.py:18: note: Revealed type is "Union[django.db.models.base.Model, builtins.list[django.db.models.base.Model]]"
test.py:19: note: Revealed type is "model_bakery.recipe.RecipeForeignKey"
test.py:20: note: Revealed type is "model_bakery.recipe.related"

whereas with this PR, more fine-grained type hinting is made available:

test.py:14: note: Revealed type is "model_bakery.recipe.Recipe[test.C*]"
test.py:15: note: Revealed type is "model_bakery.recipe.Recipe[test.C]"
test.py:16: note: Revealed type is "test.C*"
test.py:17: note: Revealed type is "test.C*"
test.py:18: note: Revealed type is "builtins.list[test.C*]"
test.py:19: note: Revealed type is "model_bakery.recipe.RecipeForeignKey[test.C*]"
test.py:20: note: Revealed type is "model_bakery.recipe.related[test.C*]"

Signed-off-by: Oleg Hoefling <oleg.hoefling@ionos.com>
Signed-off-by: Oleg Hoefling <oleg.hoefling@ionos.com>
@hoefling
Copy link
Contributor Author

hoefling commented Feb 12, 2022

Demo (pyre)

pyre doesn't support self-typing for an upper bound (which is admittedly not clearly specified by PEP 484), so it can't deduce the return type of Recipe.extend(). Everything else looks good here as well.

model_bakery/test.py:14:0 Revealed type [-1]: Revealed type for `r` is `Recipe[C]`.
model_bakery/test.py:15:0 Revealed type [-1]: Revealed type for `r.extend($parameter$spam = "eggs")` is `unknown`.
model_bakery/test.py:16:0 Revealed type [-1]: Revealed type for `r.make()` is `C`.
model_bakery/test.py:17:0 Revealed type [-1]: Revealed type for `r.make($parameter$_quantity = None)` is `C`.
model_bakery/test.py:18:0 Revealed type [-1]: Revealed type for `r.make($parameter$_quantity = 100)` is `typing.List[C]`.
model_bakery/test.py:19:0 Revealed type [-1]: Revealed type for `recipe.foreign_key(recipe.Recipe(test.C))` is `recipe.RecipeForeignKey[C]`.
model_bakery/test.py:20:0 Revealed type [-1]: Revealed type for `recipe.related(r, r)` is `related[C]`.

@timjklein36
Copy link
Collaborator

Before I kick off the CI workflows, please add an entry to the CHANGELOG.md.

@timjklein36 timjklein36 self-requested a review February 14, 2022 13:45
Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>
@hoefling
Copy link
Contributor Author

@timjklein36 done, also fixed issues reported by pre-commit.

@berinhard berinhard merged commit ab53845 into model-bakers:main Mar 31, 2022
amureki added a commit that referenced this pull request Jun 30, 2022
PR #292 introduced explicit defaults for `make` and `prepare`,
which were then overwriting custom values in certain cases (explained in #308).

This PR attempts to fix this issue by not having explicit defaults.
amureki added a commit that referenced this pull request Aug 2, 2022
* Fix #308 -- do not force defaults for `make` and `prepare`

PR #292 introduced explicit defaults for `make` and `prepare`,
which were then overwriting custom values in certain cases (explained in #308).

This PR attempts to fix this issue by not having explicit defaults.

* Update CHANGELOG.md

* Silence weird mypy assignment error

* Use Python 3 inheritance
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.

None yet

3 participants