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

Need type annotation for ManyToManyField with string class names #1802

Open
jgillard opened this issue Oct 26, 2023 · 9 comments
Open

Need type annotation for ManyToManyField with string class names #1802

jgillard opened this issue Oct 26, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@jgillard
Copy link

Bug report

What's wrong

I'm looking at updating from 4.2.4 to 4.2.6, and am seeing some new errors that I'm not sure how to handle. They're all in one single models.py file, are all ManyToManyFields, and each of those contain the related class name as a string not as the class itself. For the ones I can convert from strings to classes without having to re-order models, this fixes the error. I don't see this particular issue discussed in a any release notes / issues / MRs hence I'm asking here. Is this expected with this update, and what should I be doing about this error? The fact that I only see this in one file is suspicious to me, but we have very few other ManyToManyFields so I can't narrow it down to something breaking in this file just yet.

models.py
destinations = models.ManyToManyField('Destination')

now leads to:
error: Need type annotation for "destinations" [var-annotated]

and when I change the model to
destinations = models.ManyToManyField(Destination)

the error is gone.

How is that should be

Not sure tbh. If strings are not supported then I'd expect that to be mentioned somewhere (if it is, apologies!)

System information

  • OS: MacOS 13.6
  • python version: 3.11
  • django version: 4.2.6
  • mypy version: 1.6.1
  • django-stubs version: 4.2.6
  • django-stubs-ext version: 4.2.5
@jgillard jgillard added the bug Something isn't working label Oct 26, 2023
@flaeppe
Copy link
Member

flaeppe commented Oct 26, 2023

Lazy references are supported if you use the format "<app_label>.<model_name>". Lazy reference with implicit app label isn't supported

@jgillard
Copy link
Author

Ah OK yes that's the difference between this module and the ManyToManyFields in our other apps. Just to clarify and for the next person that finds this, implicit app name is supported by ForeignKey and OneToOneField, but not ManyToManyField? As I don't see issues with those relationships.

@cpontvieux-systra
Copy link

cpontvieux-systra commented Oct 31, 2023

Same here. I only have this error since I updated mypy from 1.5.1 to 1.6.1 and django-stubs from 4.2.4 to 4.2.6 and ONLY on ManyToManyField (not ForeignKey). Seems weird.

[Edit] Seems closely related to this #1719 change

@mschoettle
Copy link
Contributor

I have a ManyToManyField where the through model is referenced with a string class name. I needed to provide a type annotation as follows:

related_field: models.ManyToManyField[ToModel, 'ThroughModel'] = models.ManyToManyField(to=ToModel, through='ThroughModel')

Sorry, this is slightly unrelated: What's the correct type annotation for the through model when it is not a custom one?

@flaeppe
Copy link
Member

flaeppe commented Oct 31, 2023

What's the correct type annotation for the through model when it is not a custom one?

Short answer: There is no correct type annotation.

Long answer:

The plugin mimics Django's dynamic creation of the intermediate(through) model, ref: https://github.com/django/django/blob/40b3975e7d3e1464a733c69171ad7d38f8814280/django/db/models/fields/related.py#L1281-L1319

So you'll need a declaration identical to that, but once you type it out you might as well use what you type out explicitly as a through= argument.

@masarliev
Copy link

any resolution on this issue?

andersk added a commit to andersk/zulip that referenced this issue Nov 15, 2023
timabbott pushed a commit to zulip/zulip that referenced this issue Nov 15, 2023
akarsh-jain-790 pushed a commit to akarsh-jain-790/zulip that referenced this issue Nov 27, 2023
WhyNotHugo added a commit to WhyNotHugo/django-afip that referenced this issue Dec 14, 2023
@asadiqbal08
Copy link

asadiqbal08 commented Jan 9, 2024

Hello @masarliev , try with <app_label>.<model_name> e.g.

models.ManyToManyField(
  to='<app_label>.<model_name>',
  ...

@masarliev
Copy link

Hello @masarliev , try with <app_label>.<model_name> e.g.

models.ManyToManyField(
  to='<app_label>.<model_name>',
  ...

not working.
for now I changed all relations to
field: models.ManyToManyField = models.ManyToManyField.....
and works, but I don't like anotating all relations just for mypy

@jennifer-richards
Copy link

jennifer-richards commented Jan 9, 2024

not working.

Did you do the same for through parameters as well? This fixed the issue for me without annotations, but I had to do both to and through.

jennifer-richards added a commit to jennifer-richards/datatracker that referenced this issue Jan 9, 2024
django-stubs requires "app.model" instead of just "model" for
ManyToManyField lazy model references.

See typeddjango/django-stubs#1802
rjsparks pushed a commit to ietf-tools/datatracker that referenced this issue Jan 10, 2024
* chore: Unpin django-stubs / update mypy

* test: Use "app.model" for ManyToManyField

django-stubs requires "app.model" instead of just "model" for
ManyToManyField lazy model references.

See typeddjango/django-stubs#1802
dehnert added a commit to tech-squares/squaresdb that referenced this issue Jul 8, 2024
Per
typeddjango/django-stubs#1802 (comment),
`ManyToManyField`s with lazy (string) models but no app label don't
work. In this case, we can just use the class directly, not the string,
so do that.
dehnert added a commit to tech-squares/squaresdb that referenced this issue Jul 9, 2024
In newer mypy[1][2], `ManyToManyField`s with lazy (string) models but no
app label don't work. For the `to` model, we can just use `Person`
directly (and do on the line above), so do that. For the `through`
model, add the app label -- Django doesn't care, and it makes mypy
happy.

Also, in mypy, well-structured wildcard patterns match without a
submodule too, so simplify some config.

[1] typeddjango/django-stubs#1802 (comment),
[2] typeddjango/django-stubs#1719 (comment)
[3] https://mypy.readthedocs.io/en/stable/config_file.html#config-file-format
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

No branches or pull requests

7 participants