Re: HttpResponse headers interface

2020-07-14 Thread Carlton Gibson
I think we should keep the old interface.

The BC concerns are one point: it's every bit of Django code ever written. 
To keep __setitem__  is a small price to pay not to needlessly break 
that code. 

Beyond that though, the proposed API is very nice, but the pendulum will 
swing back: having response objects conform to the Python Data Model, by 
looking like dicts, will again come to seem desirable. In 
the meantime it'll be merely be handy that you can so use them — The number of times, "I can just..." will pay off here (IMO) 
tells further against removing those special methods. 

Kind Regards,

Carlton


On Tuesday, 14 July 2020 16:07:15 UTC+2, Jon Dufresne wrote:
>
> > I don't see a reason to deprecate it at all just now (though perhaps in 
> _my_ ideal world that would happen at some point), but I'm not sure if it's 
> worth keeping the current interface in the documentation at all?
>
> IMHO, we should eventually take the advice from the zen of Python "There 
> should be one-- and preferably only one --obvious way to do it.". While we 
> should not take this as dogma, I do think it is generally wise.
>
> If there are increased concerns about existing projects, perhaps we could 
> delay the initial deprecation or apply some kind of extended deprecation 
> period that would allow projects more time to migrate. Removing the old 
> interface from the docs is a great first step.
>
> But ultimately, I think having two interfaces to solve the same tasks 
> confuses new library users and makes project coding styles more difficult 
> than necessary.
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3c3a2300-59a8-4267-8dbc-2403622b5a7ao%40googlegroups.com.


Re: HttpResponse headers interface

2020-07-14 Thread Jon Dufresne
> I don't see a reason to deprecate it at all just now (though perhaps in
_my_ ideal world that would happen at some point), but I'm not sure if it's
worth keeping the current interface in the documentation at all?

IMHO, we should eventually take the advice from the zen of Python "There
should be one-- and preferably only one --obvious way to do it.". While we
should not take this as dogma, I do think it is generally wise.

If there are increased concerns about existing projects, perhaps we could
delay the initial deprecation or apply some kind of extended deprecation
period that would allow projects more time to migrate. Removing the old
interface from the docs is a great first step.

But ultimately, I think having two interfaces to solve the same tasks
confuses new library users and makes project coding styles more difficult
than necessary.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CADhq2b6qijpNCCv_M17%2B11RTUGG-MA3AG%3Dhe3RccMrciV%3DNHTA%40mail.gmail.com.


Re: HttpResponse headers interface

2020-07-14 Thread Tobias Kunze
Hi all,

first off: Thank you for your work, Tom, this will be one of the changes that
I will start using immediately and then wonder how I got by without.

>One further small addition, I think it would be good to be able to pass
>> headers into the HttpResponse object
>
>I'm also behind that. A quick survey shows that Flask, Pyramid, and
>Starlette all allow passing headers in response construction.

Absolutely this. I always found the response[header_name] approach odd.
It gave me the feeling that Django responses are very low-level and meddling
with them was a bit hacky and not intended.

Tobias

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20200714122156.7jygzocgwjhd6khu%40cordelia.localdomain.


signature.asc
Description: PGP signature


Re: HttpResponse headers interface

2020-07-14 Thread Adam Johnson
>
> I'm wondering though, if we should prefer this interface over the old one,
> as it's a bit more explicit in my view. I'd be happy to go through the docs
> and change the examples to using the headers attribute directly.
>

I think it's preferable. The old interface was so unclear.

It would also be good to update as many of Django's tests as possible, at
least those that can be found with a regex, e.g. resp(onse)?\[['"] .


> don't see a reason to deprecate it at all just now (though perhaps in _my_
> ideal world that would happen at some point)


Yes, this will probably need to live 'forever' for backwards compatibility
concerns. The same as for request.GET etc. as per
https://groups.google.com/d/msg/django-developers/Kx8BfU-z4_E/gJBuGeZTBwAJ .

One further small addition, I think it would be good to be able to pass
> headers into the HttpResponse object
>

I'm also behind that. A quick survey shows that Flask, Pyramid, and
Starlette all allow passing headers in response construction. I think a
separate ticket is warranted, it would require updating yet more
docs/tests, so it would be good to merge step one before embarking on step
two.

On Tue, 14 Jul 2020 at 12:58, Tom Carrick  wrote:

> I've pushed a proof of concept here:
> https://github.com/django/django/pull/13186
>
> I decided to do a bit more than I initially intended. It seems like the
> headers can be stored as a public interface and single source of truth now,
> rather than just adding an extra public property based on it, by using a
> slightly modified CaseInsensitiveMapping.
>
> Perhaps I'm missing some reason - and if there is one, let me know - that
> this shouldn't be a public, documented interface, but I'm not sure what
> that is.
>
> If this all seems good, I'll write up docs and tests (all current tests
> seem to be passing). I'm wondering though, if we should prefer this
> interface over the old one, as it's a bit more explicit in my view. I'd be
> happy to go through the docs and change the examples to using the headers
> attribute directly. I don't see a reason to deprecate it at all just now
> (though perhaps in _my_ ideal world that would happen at some point), but
> I'm not sure if it's worth keeping the current interface in the
> documentation at all?
>
> One further small addition, I think it would be good to be able to pass
> headers into the HttpResponse object, so rather than doing:
>
> response = HttpResponse()
> response['foo'] = 'bar'
> return response
>
> You could instead return HttpResponse(headers={'foo': 'bar'})
>
> Perhaps that's better in a separate ticket / PR, but it seems like a
> minimal amount of effort to add it at the same time.
>
> Cheers,
> Tom
>
> On Wed, 17 Jun 2020 at 21:53, Adam Johnson  wrote:
>
>> I have also found this a little odd when writing tests. It would
>> certainly make it easier to write both normal Django code and tests, and
>> it's a small addition, so +1 from me.
>>
>> On Wed, 17 Jun 2020 at 15:35, Tom Carrick  wrote:
>>
>>> I don't find myself using HttpResponse very often, usually I'm using
>>> DRF's Responses. But today I needed to use one, and as I was writing tests,
>>> I ended up somewhat astonished, so with the principle of least astonishment
>>> in mind... I had anticipated that I could check the headers with
>>> `response.headers`, similar to how the new request.headers
>>>  works, but apparently
>>> this is not the case. After reading the docs, I found out that I should
>>> just treat the HttpResponse object itself as if it were a dictionary of
>>> headers. This seems very strange to me, it's not what I expect, but maybe
>>> I'm in the minority.
>>>
>>> I have no interest in deprecating the old API, but it would be nice if
>>> the headers were all accessible from a simple headers dict, and perhaps
>>> make this the source of truth, allowing access with any casing but
>>> preserving the original casing for output. It looks like what is currently
>>> HttpResponse._headers was once HttpRequest.headers, but this was 13
>>> years ago , I don't think
>>> it'd be confusing to add the property back as something different.
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to django-developers+unsubscr...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/django-developers/CAHoz%3DMa9-m%2Bfqj0wqzQ7qW5Aiw3POHtNOp2NTBaHeP_ux5FhLg%40mail.gmail.com
>>> 
>>> .
>>>
>>
>>
>> --
>> Adam
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers 

Re: HttpResponse headers interface

2020-07-14 Thread Tom Carrick
I've pushed a proof of concept here:
https://github.com/django/django/pull/13186

I decided to do a bit more than I initially intended. It seems like the
headers can be stored as a public interface and single source of truth now,
rather than just adding an extra public property based on it, by using a
slightly modified CaseInsensitiveMapping.

Perhaps I'm missing some reason - and if there is one, let me know - that
this shouldn't be a public, documented interface, but I'm not sure what
that is.

If this all seems good, I'll write up docs and tests (all current tests
seem to be passing). I'm wondering though, if we should prefer this
interface over the old one, as it's a bit more explicit in my view. I'd be
happy to go through the docs and change the examples to using the headers
attribute directly. I don't see a reason to deprecate it at all just now
(though perhaps in _my_ ideal world that would happen at some point), but
I'm not sure if it's worth keeping the current interface in the
documentation at all?

One further small addition, I think it would be good to be able to pass
headers into the HttpResponse object, so rather than doing:

response = HttpResponse()
response['foo'] = 'bar'
return response

You could instead return HttpResponse(headers={'foo': 'bar'})

Perhaps that's better in a separate ticket / PR, but it seems like a
minimal amount of effort to add it at the same time.

Cheers,
Tom

On Wed, 17 Jun 2020 at 21:53, Adam Johnson  wrote:

> I have also found this a little odd when writing tests. It would certainly
> make it easier to write both normal Django code and tests, and it's a small
> addition, so +1 from me.
>
> On Wed, 17 Jun 2020 at 15:35, Tom Carrick  wrote:
>
>> I don't find myself using HttpResponse very often, usually I'm using
>> DRF's Responses. But today I needed to use one, and as I was writing tests,
>> I ended up somewhat astonished, so with the principle of least astonishment
>> in mind... I had anticipated that I could check the headers with
>> `response.headers`, similar to how the new request.headers
>>  works, but apparently this
>> is not the case. After reading the docs, I found out that I should just
>> treat the HttpResponse object itself as if it were a dictionary of headers.
>> This seems very strange to me, it's not what I expect, but maybe I'm in the
>> minority.
>>
>> I have no interest in deprecating the old API, but it would be nice if
>> the headers were all accessible from a simple headers dict, and perhaps
>> make this the source of truth, allowing access with any casing but
>> preserving the original casing for output. It looks like what is currently
>> HttpResponse._headers was once HttpRequest.headers, but this was 13
>> years ago , I don't think
>> it'd be confusing to add the property back as something different.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/CAHoz%3DMa9-m%2Bfqj0wqzQ7qW5Aiw3POHtNOp2NTBaHeP_ux5FhLg%40mail.gmail.com
>> 
>> .
>>
>
>
> --
> Adam
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMyDDM2HCJKbW%3D3dNkB7BDV4R6eSAGokg%3DPENFd%3D%2BhNwaTt9OQ%40mail.gmail.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAHoz%3DMa7QfRHgaT7iJTAqnpzVo6ZoHKcdLVp8dER%2BCus0G_wpw%40mail.gmail.com.


Re: Admin accessibility

2020-07-14 Thread Thibaud Colas
 it’s wonderful to see this happening! Re-reading through the whole 
thing, as Tobias mentioned I also find it very easy to read, and makes a 
good case.

On Tuesday, 14 July 2020 at 09:24:15 UTC+1 t...@carrick.eu wrote:

> I've added a PR to the DEPs repo to hopefully get some eyes on it: 
> https://github.com/django/deps/pull/69
>
> Thibaud, I think whatever you have the time and motivation for sounds 
> good, all of those things are useful. If you're not sure about all the 
> admin features, I think that's pretty normal. One project I've had on my 
> mind for a while now is to build a simple django site that is essentially 
> just there to use every feature of the admin, so I might bump that up the 
> priority list, though it's somewhat daunting.
>
> Cheers,
> Tom
>
> On Mon, 13 Jul 2020 at 01:15, Thibaud Colas  wrote:
>
>> Update for the proof of concept CI tests from my side – thank you Tom for 
>> the feedback. Here are the latest additions to the test suite & reports, 
>> still living at https://thibaudcolas.github.io/django_admin_tests/,
>>
>> - Added as much as I know about in the admin, and now also outside of it 
>> a bit (startproject welcome page, error pages)
>> - Separated the issues between Axe and HTML_CS so the numbers are easier 
>> to understand
>> - Added anchor links everywhere for easier navigation
>> - I’ve also started a draft list of "things to (potentially) audit", over 
>> at 
>> https://github.com/thibaudcolas/django_admin_tests#scope-for-future-audits
>>
>> I think the next two big steps are what you mention:
>>
>> - Having a way to track the number of issues over time. Currently the 
>> report overwrites itself every week (well, every build). If you have a 
>> suggestion on ways to demo this that would be useful please let me know. 
>> Currently I’m thinking sparklines for each test case could be nice as a 
>> proof of concept, and a sparkline for the total number of issues. Or see 
>> whether I should get out of my comfort zone a bit and find a 
>> dashboard/graphing tool to send the metrics to and graph there.
>> - Testing more features of modeladmin. I don’t use it too frequently 
>> myself so don’t really know what’s “easy” to enable – if you know of an 
>> existing test suite I could repurpose, or of an example of using a lot of 
>> functionality – I’d be keen to invest time to add it to my test site.
>>
>> Alternatively something else I could do is to file a ticket for 
>> accessibility issues with the welcome page – I’ve tested it with screen 
>> readers, there are a few issues, but nothing that should be too hard to 
>> fix, and it might be a good demo of what reporting accessibility issues in 
>> general could look like?
>>
>> Cheers,
>>
>> Thibaud
>>
>>
>> On Tuesday, 30 June 2020 at 18:59:53 UTC+1 Tobias Bengfort wrote:
>>
>>> Nice writeup! I found it easy to read (I did not catch myself skipping 
>>> paragraphs which is always a good sign). Contentwise, I would have no 
>>> issue if this was accepted as is. Maybe I am forgetting about some 
>>> important details though. 
>>>
>>> I had just some ideas that might be good additions: 
>>>
>>> - mention ATAG somewhere 
>>>
>>> - It would be nice to have a real commitment to accessibility. Something 
>>> along the lines of "accessibility bugs must be treated with the same 
>>> priority as any other bugs". 
>>>
>>> - The step from "leaving accessibility in the hands of 
>>> individual contributors" to "you have to commit for 9 months" is a tad 
>>> big. I keep wondering if we can do something to improve the options in 
>>> between those. One idea would be to formalize an "a11y" keyword so 
>>> interested contributors can easily find related tickets. 
>>>
>>> tobias 
>>>
>> -- 
>>
> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-develop...@googlegroups.com.
>>
> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/e65e3879-d74c-4401-9232-29eb0a73385cn%40googlegroups.com
>>  
>> 
>> .
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/6e323a74-38a7-4c64-9b22-4c71c6410056n%40googlegroups.com.


Re: Admin accessibility

2020-07-14 Thread Tom Carrick
I've added a PR to the DEPs repo to hopefully get some eyes on it:
https://github.com/django/deps/pull/69

Thibaud, I think whatever you have the time and motivation for sounds good,
all of those things are useful. If you're not sure about all the admin
features, I think that's pretty normal. One project I've had on my mind for
a while now is to build a simple django site that is essentially just there
to use every feature of the admin, so I might bump that up the priority
list, though it's somewhat daunting.

Cheers,
Tom

On Mon, 13 Jul 2020 at 01:15, Thibaud Colas  wrote:

> Update for the proof of concept CI tests from my side – thank you Tom for
> the feedback. Here are the latest additions to the test suite & reports,
> still living at https://thibaudcolas.github.io/django_admin_tests/,
>
> - Added as much as I know about in the admin, and now also outside of it a
> bit (startproject welcome page, error pages)
> - Separated the issues between Axe and HTML_CS so the numbers are easier
> to understand
> - Added anchor links everywhere for easier navigation
> - I’ve also started a draft list of "things to (potentially) audit", over
> at
> https://github.com/thibaudcolas/django_admin_tests#scope-for-future-audits
>
> I think the next two big steps are what you mention:
>
> - Having a way to track the number of issues over time. Currently the
> report overwrites itself every week (well, every build). If you have a
> suggestion on ways to demo this that would be useful please let me know.
> Currently I’m thinking sparklines for each test case could be nice as a
> proof of concept, and a sparkline for the total number of issues. Or see
> whether I should get out of my comfort zone a bit and find a
> dashboard/graphing tool to send the metrics to and graph there.
> - Testing more features of modeladmin. I don’t use it too frequently
> myself so don’t really know what’s “easy” to enable – if you know of an
> existing test suite I could repurpose, or of an example of using a lot of
> functionality – I’d be keen to invest time to add it to my test site.
>
> Alternatively something else I could do is to file a ticket for
> accessibility issues with the welcome page – I’ve tested it with screen
> readers, there are a few issues, but nothing that should be too hard to
> fix, and it might be a good demo of what reporting accessibility issues in
> general could look like?
>
> Cheers,
>
> Thibaud
>
>
> On Tuesday, 30 June 2020 at 18:59:53 UTC+1 Tobias Bengfort wrote:
>
>> Nice writeup! I found it easy to read (I did not catch myself skipping
>> paragraphs which is always a good sign). Contentwise, I would have no
>> issue if this was accepted as is. Maybe I am forgetting about some
>> important details though.
>>
>> I had just some ideas that might be good additions:
>>
>> - mention ATAG somewhere
>>
>> - It would be nice to have a real commitment to accessibility. Something
>> along the lines of "accessibility bugs must be treated with the same
>> priority as any other bugs".
>>
>> - The step from "leaving accessibility in the hands of
>> individual contributors" to "you have to commit for 9 months" is a tad
>> big. I keep wondering if we can do something to improve the options in
>> between those. One idea would be to formalize an "a11y" keyword so
>> interested contributors can easily find related tickets.
>>
>> tobias
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/e65e3879-d74c-4401-9232-29eb0a73385cn%40googlegroups.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAHoz%3DMZ_%2BR%2BMWxeJtsQt1%3DW-9hT-wS7Nk7ALUKnozDaRKzQFbw%40mail.gmail.com.