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

Wrong typehinting in Recipe init #124

Closed
GitRon opened this issue Oct 28, 2020 · 12 comments
Closed

Wrong typehinting in Recipe init #124

GitRon opened this issue Oct 28, 2020 · 12 comments

Comments

@GitRon
Copy link
Contributor

GitRon commented Oct 28, 2020

The typehinting in the init method of Recipe is wrong. It states that _model is supposed to be str - but it can be an instance of django model as well.

Expected behavior

from typing import Union
from django.db import models
class Recipe(object):
    def __init__(self, _model: Union[str, models.Model], **attrs) -> None:

Actual behavior

class Recipe(object):
    def __init__(self, _model: str, **attrs) -> None:

Reproduction Steps

PyCharm will always show an error if I pass a model class to the Recipe. That is quite annoying and misleading.

Versions

Python: 3.8
Django: 3.1.2
Model Bakery: 1.2.0

Thanks!

@palfrey
Copy link
Contributor

palfrey commented Oct 29, 2020

I think this is fixed in #115 (which isn't released yet)

@GitRon
Copy link
Contributor Author

GitRon commented Oct 29, 2020

I think this is fixed in #115 (which isn't released yet)

Would be awesome! Any idea when the release is going to happen?

@berinhard
Copy link
Member

Hi @GitRon! I'll release a new version today =)

@GitRon
Copy link
Contributor Author

GitRon commented Nov 12, 2020

@berinhard Did you? 😅

@berinhard
Copy link
Member

Yep! I was waiting for PR #126 to be merged. 1.2.1 is now live on pypi:

https://pypi.org/project/model-bakery/1.2.1/

@GitRon
Copy link
Contributor Author

GitRon commented Nov 13, 2020

Awesome! But my PyCharm is still complaining. I think

_model: Union[str, Type[ModelBase]]

should be

_model: Union[str, Type[Model]]

What do you think @berinhard ?

Best
Ronny

GitRon pushed a commit to GitRon/model_bakery that referenced this issue Nov 13, 2020
@GitRon
Copy link
Contributor Author

GitRon commented Nov 13, 2020

@berinhard I create a PR to tackle the issue.

@berinhard berinhard reopened this Nov 16, 2020
@berinhard
Copy link
Member

Thanks @GitRon! I added a comment in the PR

GitRon pushed a commit to GitRon/model_bakery that referenced this issue Nov 16, 2020
@GitRon
Copy link
Contributor Author

GitRon commented Nov 16, 2020

@berinhard Awesome! I pushed a change on the changelog. Hope the pipeline will succeed 😅

berinhard pushed a commit that referenced this issue Nov 23, 2020
* #124: Corrected type hint for recipe param "_model"

* #124: Added changelog entry
@GitRon
Copy link
Contributor Author

GitRon commented Nov 30, 2020

@berinhard Any chance that my fix will be pushed to pypi soon? ❤️

@berinhard
Copy link
Member

berinhard commented Nov 30, 2020

@GitRon I don't think that I'll do that in the next days, sorry =S

If this is a deal breaker for you, I highly recommend you to install the project using the main branch in Github. I'm saying that because I'm planning to implement at least one of the following issues before a new release:

#25
#26
#65
#97

@GitRon you're very welcome to help me with any of these! @anapaulagomes @amureki and @timjklein36 too =)

@GitRon
Copy link
Contributor Author

GitRon commented Nov 30, 2020

Alright, if you have a plan, I can live with that. You are the boss here 😉

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

No branches or pull requests

3 participants