Update @property attributes to @cached_property part 1

[UnknownPlatypus]

Trimming down the todo list.

This adds the missing methods to django-stubs/http/request.pyi and also replace a bunch of @property that are now @cached_property

This removes 69 entries from the todolist (nice)

[UnknownPlatypus]

ALready defined correctly in inherited FileSystemStorage

[intgr]

Hrm, looking at StaticFilesStorage.__init__, these could be cached_property or instance attribute None (if settings.STATIC_ROOT is unset). I'm not sure that can be typehinted accurately.

But we should add None to possible values, up to you if you want to do it in this PR.

[UnknownPlatypus]

Django will raise if self.location is not set so I don't know if we should add None t possible values ?

[sobolevn]

Thank you!

[UnknownPlatypus]

When trying to to the same for other @property that are now @cached_property, I faced the following issue:

class Transform:
    ...
-   @property
+   @cached_property
    def output_field(self) -> Field: ...

class LowerInclusive(Transform):
    output_field: models.BooleanField  # error:  Incompatible types in assignment (expression has type "BooleanField[Any, Any]", base class "BaseExpression" defined the type as "Callable[[BaseExpression], Field[Any, Any]]")

class RangeStartsWith(Transform):
    @property
    def output_field(self) -> models.Field: ... # error: Signature of "output_field" incompatible with supertype "BaseExpression"

From what I've seen, Django often override a @property with a class variable on the subclass because the value is constant.
This was somehow not causing issues with a @property, but is now with a @cached_property.

So I'm a bit confused and not sure what we should be doing here but I feel like the @cached_property decorator will make it really hard for users to override properties without triggering mypy issues.

[flaeppe]

In my opinion it's fine to output_field: ClassVar[Field] # type: ignore[assignment] those.

And in regards to other users, I suppose it'll be up to them to chose keeping it a cached_property or having a class variable.

This is just a hunch, but I'm getting a feeling that Django went with using class vars to spare a couple of lines declaring a property.

A suggestion for a next place to change could be updating django.db.models.expressions.BaseExpression.output_field to a @cached_property. That would probably spread a bit.

[UnknownPlatypus]

And in regards to other users, I suppose it'll be up to them to chose keeping it a cached_property or having a class variable.

Maybe it's actually a good thing, because overriding a @cached_property with a @property is probably never what you really want, if Django thinks it requires a @cached_property in the base class, it's probably for a reason. I wonder if the cases I found in Django himself are not mistakes ?

This is just a hunch, but I'm getting a feeling that Django went with using class vars to spare a couple of lines declaring a property.

Maybe, but it should also be better performance wise, so for something constant, the @cached_property seems overkill since the attribute access on the instance behaves the same in both cases.

A suggestion for a next place to change could be updating django.db.models.expressions.BaseExpression.output_field to a @cached_property. That would probably spread a bit.

Ye, that's while doing that I encountered the issue above, I will update my other branch. I think what you said above makes sense. output_field: ClassVar[Field] # type: ignore[assignment] is the most accurate typing for these.

[intgr]

Sure, this fixes lots of "todolist" entries, but is this change really desired? Some data points:

• We have multiple examples where Django interchangeably uses and overrides @property, @cached_property, ClassVar and instance attributes.
• Mypy even with --strict allows overriding @property and @functools.cached_property with classvar, and @functools.cached_property with instance attribute (mypy-play example)
• Mypy's special handling of @functools.cached_property does not extend to Django's django.utils.functional.cached_property. Mypy does not allow overriding Django's @cached_property with @functools.cached_property, so users must pay attention where they import cached_property from.

So both Django and mypy have decided that properties, cached properties and classvars are interchangeable. But this PR change would disallow that for django-stubs users, and force strictly distinguishing cached_properties from others.

To me that seems inconsistent and overly strict.

Maybe we can find a way to relax django.utils.functional.cached_property? Could we get away with making it an alias of functools.cached_property for typing purposes and inherit mypy's special handling also?

[intgr]

This is unblocked, the prerequisite PR #1771 is now merged

[intgr]

Thanks, looks good. Sorry for the delay.