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

Add type-checking to Manager.acreate #2477

Merged
merged 3 commits into from
Jan 10, 2025

Conversation

vitosamson
Copy link
Contributor

@vitosamson vitosamson commented Jan 10, 2025

This PR adds type-checking to kwargs for the .acreate() method. Currently .create() is type-checked but .acreate() is not.

Related issues

Closes #2090

This is my first time diving into this codebase so let me know if I missed anything. I used the test case from #2093.

@vitosamson vitosamson changed the title add type-checking to Manager.acreate Add type-checking to Manager.acreate Jan 10, 2025
@vitosamson vitosamson marked this pull request as ready for review January 10, 2025 16:41
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! One test suggestion

import asyncio
from myapp.models import User
async def amain() -> None:
await User.objects.acreate(pk=1, name='Max', age=10)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await User.objects.acreate(pk=1, name='Max', age=10)
reveal_type(await User.objects.acreate(pk=1, name='Max', age=10))

@sobolevn sobolevn merged commit 5bff9f8 into typeddjango:master Jan 10, 2025
40 checks passed
@vitosamson
Copy link
Contributor Author

Thanks @sobolevn! Any idea when this might go out in a release?

@sobolevn
Copy link
Member

We do releases every couple of month. I think that somewhere before / around the start of Feb.

@sobolevn
Copy link
Member

@flaeppe oh, I double checked. Our latest release was in Oct. So, maybe we can do one?

@vitosamson
Copy link
Contributor Author

That would be great since this issue caused us to miss a bug. I saw in the contributing docs that anybody can open the release PR - I'm happy to get that ball rolling if it would help.

@sobolevn
Copy link
Member

@vitosamson please, go ahead :)

@vitosamson
Copy link
Contributor Author

@sobolevn done #2478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Manager.acreate: model isn't type checked
2 participants