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

Manager.bulk_update expects fields to be Sequence[str] #1195

Closed
kezabelle opened this issue Oct 19, 2022 · 3 comments · Fixed by #2296
Closed

Manager.bulk_update expects fields to be Sequence[str] #1195

kezabelle opened this issue Oct 19, 2022 · 3 comments · Fixed by #2296
Labels
bug Something isn't working

Comments

@kezabelle
Copy link

The bulk_update definition for BaseManager is:

def bulk_update(self, objs: Iterable[_T], fields: Sequence[str], batch_size: Optional[int] = ...) -> int: ...

Note: fields: Sequence[str]

which AFAIK is incorrect, and has already been fixed for _QuerySet in:

def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> int: ...

Note: fields: Iterable[str]


It seems to me that they should both be the same, and that for example {"a", "b", "c"} (a Set[str]) should pass on both (because internally to Django, the fields is consumed by iteration and re-cast to a list of Field instances IIRC)

@kezabelle kezabelle added the bug Something isn't working label Oct 19, 2022
@adamchainz
Copy link
Contributor

Yup seems right, would you like to make a PR?

We could probably do with a test that methods are in sync between BaseManager and _QuerySet.

@q0w
Copy link
Contributor

q0w commented Oct 21, 2022

#1114 covers it

@kezabelle
Copy link
Author

Good eye 🔍
Happy to close this as a duplicate if that PR is going to solve the issue (especially as I haven't gotten around to opening a PR myself yet; the eternal backlog 😄)

As a data point, though I think it unrelated, I encountered the typing discrepancy on a 3.2 project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants