Typing of TranslationOptions.fields

[mschoettle]

I am trying to update to the latest version but there are some typing problems:

Given the following TranslationOptions for a model Foo:

@register(Foo)
class FooTranslationOptions(TranslationOptions):
    fields = ('name', 'description')

This is inline with the documentation.

However, mypy raises the following error:

translation.py:15: error: Incompatible types in assignment (expression has type "tuple[str, str]", base class "TranslationOptions" defined the type as "dict[str, set[TranslationField]]") [assignment]

Is this an issue with the type hint in TranslationOptions or is there another way to define fields?

[mschoettle]

The problem is that self.fields is expected to be an Iterable[str] and assigned to the same variable as another type:

https://github.com/deschler/django-modeltranslation/blob/47dcb9cd460c9a26ab98ccf6344ad899a4721d09/modeltranslation/translator.py#L102

Can it be assigned to a new variable? Does this affect other places where TranslationOptions.fields is accessed?

cc: @Viicos

[last-partizan]

That's a tricky case. (But pyright, somehow, gets it right)

Probably, it can be assigned to a new variable, but it feels wrong to change code only to satisfy type checker. And we need to change it everywhere where it's used.

[last-partizan]

But pyright, somehow, gets it right

Actually, no. pyright does not raise error when setting fields, but later it treats field as Iterable[str], which is wrong.

[last-partizan]

Please, test the solution from https://github.com/deschler/django-modeltranslation/pull/739

[Viicos]

Yes this is unfortunate, we are reaching the limits of typing support for metaprogramming here. Assuming the instance value of fields is private API, maybe we could:

• Create a new self._fields attribute, correctly typed as dict[str, set[TranslationField]], that would be used internally so that we don't face typing errors in the source code.
• Leave self.fields as is for backwards compatibility in case someone's using it even though it might be uncommon. This will probably require a type ignore comment

[Viicos]

Edit: saw your open PR and related discussion, probably best to go your way

[last-partizan]

@Viicos i really hope we can do something like this: https://discuss.python.org/t/support-different-type-for-class-variable-and-instance-variable/54198/5 (but no luck so far).

If we can't get it to work, i'd probably merge my PR with Any as fallback. This way we won't break public API and won't need to change much code.

[last-partizan]

In the end, i renamed .fields to .all_fields. Now public fields is of correct Sequence[str] type, and we have separate private all_fields property.

[mschoettle]

@last-partizan Thanks a lot for fixing this so quickly. I'll try to work on fixing my PR (#737) soon to also fix the admin generic typing