Fixed TemplateResponse.cookies attribute and SimpleCookie type

[apollo13]

I have made things!

This definition makes it usable even in pyright's strict mode. See microsoft/pyright#5927 for details.

Related issues

None that I know off.

@adamchainz Does that make sense? :)

[flaeppe]

django.http.SimpleCookie is mentioned as backwards compatibility for 2.1. While the update here is correct. I'd suggest using http.cookies.SimpleCookie instead

[apollo13]

@flaeppe What do you mean? Remove it from cookies.pyi and directly reference it in response.pyi?

[sobolevn]

SimpleCookie is defined as generic in typeshed: https://github.com/python/typeshed/blob/32535ee8952dffecbe7a9fd04e5ed8ba0f1c8da2/stdlib/http/cookies.pyi#L60

So, let's make it generic here as well:

_T = TypeVar("_T")

SimpleCookie: TypeAlias = cookies.SimpleCookie[_T]

[apollo13]

If I use a typevar then pyright complains:

  /home/florian/sources/unpoly/python-unpoly/test.py:7:13 - error: Type of "cookies" is partially unknown
    Type of "cookies" is "SimpleCookie[Unknown]" (reportUnknownMemberType)

[sobolevn]

Add a typevar to your code as well :)
Or a specific type argument.

[flaeppe]

@flaeppe What do you mean? Remove it from cookies.pyi and directly reference it in response.pyi?

I wasn't aware that it was used by other parts of the stubs. Do we really have to follow the alias for backwards compatibility in other parts of the stubs? All we really do is lose type information.

e.g. template/response.pyi uses http.cookies.SimpleCookie

I think we should declare http.cookies.SimpleCookie[str] for HttpResponseBase in http.response?

[apollo13]

How would that look like. My code simply tries to do:

response.cookies["abc"].value

Want me to add an explicit cast around that? To be honest, the way cookies is used, Any is as good as any.

[flaeppe]

I think we should declare http.cookies.SimpleCookie[str] for HttpResponseBase in http.response?

Reason being that we've declared:

django-stubs/django-stubs/http/response.pyi

Lines 56 to 58 in f5e65d2

	def set_cookie(
	self,
	key: str,

And

django-stubs/django-stubs/http/response.pyi

Line 69 in f5e65d2

def set_signed_cookie(self, key: str, value: str, salt: str = ..., **kwargs: Any) -> None: ...

i.e. key: str correlating to the type argument of SimpleCookie

[apollo13]

Does key: str really belong to the type argument? As far as I can tell that is passed down into Morsel and only used for coded_value, the key property is always str: https://github.com/python/typeshed/blob/32535ee8952dffecbe7a9fd04e5ed8ba0f1c8da2/stdlib/http/cookies.pyi#L31

But I have to admit that I do not know typing well enough (nor what coded_value actually is).

[flaeppe]

Oh, I see, my mistake. That's interesting, the documentation says this

Morsel.coded_value
The encoded value of the cookie — this is what should be sent.

But I only see that it's adjustable via Morsel.set. And Django only seem to use __setitem__. I guess it's SimpleCookie[Any] then, unless we can draw some other conclusion from value: str?

[flaeppe]

Or it's actually SimpleCookie[str] from the fact that Morsel.set isn't used. Might be even more true, at least right now.

[apollo13]

I have no idea, both are fine for me. Just tell me if you want me to adjust the MR to str and if/what I should to with templates/response.pyi. Should they both import from django.http.cookies or should we utilize http.cookies from the stdlib directly.

[sobolevn]

Suggested change

	from http import cookies
	from http.cookies import SimpleCookie as SimpleCookie

[apollo13]

That does not fix the issue though since then it will still we generic.

[flaeppe]

I have no idea, both are fine for me. Just tell me if you want me to adjust the MR to str and if/what I should to with templates/response.pyi. Should they both import from django.http.cookies or should we utilize http.cookies from the stdlib directly.

I think we should use SimpleCookie[str] on HttpResponseBase too. I also find it interesting that it's a TypeVar on SimpleCookie in typeshed, since the implementation seem to use string?

Ref: https://github.com/python/cpython/blob/1ee50e2a78f644d81d341a08562073ad169d8cc7/Lib/http/cookies.py#L607-L612

The documentation also only talks about strings regarding SimpleCookie

Ref: https://docs.python.org/3/library/http.cookies.html#http.cookies.SimpleCookie

But perhaps there's something else that I'm missing.

[adamchainz]

It may not be that typeshed is correct with the Generic there, it was added a long time ago in a "big updates" commit with little review: python/typeshed#248 .

[flaeppe]

I see, that explains it a bit, for the curious one I opened these 2 issues:

• value_encode and value_decode of http.cookies.BaseCookie not returning tuples python/typeshed#10699
• Declare http.cookies.SimpleCookie as non generic(?) python/typeshed#10700

[apollo13]

Ok, let's wait for a resolution in typeshed first before changing here and then changing it back again?

…

On Mon, Sep 11, 2023, at 22:51, Petter Friberg wrote: I see, that explains it a bit, for the curious one I opened these 2 issues: • python/typeshed#10699 <python/typeshed#10699> • python/typeshed#10700 <python/typeshed#10700> — Reply to this email directly, view it on GitHub <#1702 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAT5C66JMJ6YF5BRF7XVBTXZ52UNANCNFSM6AAAAAA4TY2NUE>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[apollo13]

Should we keep this or simply remove it?

[intgr]

Let's keep it as long as Django has this alias.

[apollo13]

I have updated the PR to mostly use http.cookies.SimpleCookie directly. I think this would be the preferred approach now?

[sobolevn]

We need to wait for the next mypy release

[intgr]

We need to wait for the next mypy release

Mypy 1.6 was released last week and we have updated to it. Is this PR unblocked now?

[flaeppe]

Not at all sure how typeshed is released really, but this is the relevant commit. If that's useful: python/typeshed@c9bf034

[intgr]

Ah OK. The typeshed stdlib stubs aren't "released" per se, every mypy release includes a vendored copy of typeshed.

Unfortunately this change didn't make the cut for mypy 1.6: https://github.com/python/mypy/blob/v1.6.0/mypy/typeshed/stdlib/http/cookies.pyi#L60

But I heard mypy 1.7 should be out in a few weeks.

[intgr]

Mypy 1.7 will include this fix https://github.com/python/mypy/blob/release-1.7/mypy/typeshed/stdlib/http/cookies.pyi#L60

• Release 1.7 planning python/mypy#16341