make wsgiref.headers.Headers accept empty constructor

[tarekziade]

BPO	5800
Nosy	@pjeby, @tarekziade, @merwok, @bitdancer, @berkerpeksag
Files	patch issue5800.diff issue5800.diff issue5800_v2.diff issue5800_v3.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/berkerpeksag'
closed_at = <Date 2014-07-02.05:38:54.044>
created_at = <Date 2009-04-20.16:01:04.297>
labels = ['easy', 'type-feature', 'library']
title = 'make wsgiref.headers.Headers accept empty constructor'
updated_at = <Date 2014-07-02.05:38:54.043>
user = 'https://github.com/tarekziade'

bugs.python.org fields:

activity = <Date 2014-07-02.05:38:54.043>
actor = 'berker.peksag'
assignee = 'berker.peksag'
closed = True
closed_date = <Date 2014-07-02.05:38:54.044>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2009-04-20.16:01:04.297>
creator = 'tarek'
dependencies = []
files = ['14311', '21016', '35721', '35787', '35814']
hgrepos = []
issue_num = 5800
keywords = ['patch', 'easy', 'needs review']
message_count = 26.0
messages = ['86199', '89458', '89459', '89484', '121680', '121681', '121690', '121829', '121958', '121959', '122276', '122318', '122323', '122352', '130169', '130181', '130752', '131578', '221243', '221247', '221248', '221650', '222020', '222033', '222069', '222070']
nosy_count = 10.0
nosy_names = ['pje', 'tarek', 'eric.araujo', 'r.david.murray', 'ptn', 'SilentGhost', 'BreamoreBoy', 'ramiroluz', 'python-dev', 'berker.peksag']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue5800'
versions = ['Python 3.5']

[tarekziade]

wsgiref.headers.Headers will let you manage a collection of HTTP
response headers, but the constructor forces you to provide a list:

>>> from wsgiref.headers import Headers
>>> headers = Headers([])
>>> headers.add_header('Content-Type', 'text/plain')

A logical change would be to allowed creating an empty Headers instance:

>>> from wsgiref.headers import Headers
>>> headers = Headers()
>>> headers.add_header('Content-Type', 'text/plain')

[ptn]

Patch attached. While I was at it, I also removed stupid whitespace and
generally made the module more PEP-8-compliant.

[tarekziade]

I have added another issue for PEP-8 compliancy at bpo-5801

[ptn]

Added a pointer from bpo-5801 to here.

[SilentGhost]

Correct and update patch + update test case

[SilentGhost]

here is the updated test case

[ramiroluz]

I applied the patches for wsgiref.headers and test_headers.py, ran the test with runtests.sh test_headers.py and it passed.

I also tried the code in the description:

>> from wsgiref.headers import Headers
>> headers = Headers()
>> headers.add_header('Content-Type', 'text/plain')
>>

[merwok]

FTR, note that “svn diff file1 file2...” will give you one file for many edits. It’s easier to review and apply.

Regarding the change, I don’t know if wsgiref 3.2 has to be compatible with Python 2.1, which would exclude using the ternary operation.

The change from type to isinstance will probably be rejected, since WSGI does not accept subclasses for the things it defines.

[pjeby]

Yes, please consider the type->isinstance part of the change rejected. I just got done reverting a bunch of those in 3.2. Where WSGI specifies types, it means "type() is", not "isinstance".

(The 3.x version of wsgiref does not need to be executable on 2.x versions, but this doesn't mean the protocol itself can be altered in backwards-incompatible ways, just the implementation.)

[SilentGhost]

Do I have to resubmit the patch or can you use the existing one?

[SilentGhost]

Re-submitting the patch for Lib/wsgiref/headers.py w/o the isinstance change

[SilentGhost]

Here is the correction for the docs. I would love to see this making it into 3.2 release.

[ramiroluz]

I reviewed the patch.

• I applied all the patchs(3 files).
• Ran make and make html in the Doc directory.
• Ran the test_wsgiref.py
• Ran the python interpreter and typed the suggested code:

>> from wsgiref.headers import Headers
>> headers = Headers([])
>> headers.add_header('Content-Type', 'text/plain')
>> headers = Headers()
>> headers.add_header('Content-Type', 'text/plain')
>>

• Read the documentation in a web browser.

All in the revision 86742.

[merwok]

LGTM.

[SilentGhost]

Here's the single-file patch against the revision.

[pjeby]

Looks good to me.

[SilentGhost]

Looks good to me.
Would you mind committing it then?

[merwok]

I think the doc should say something like “default value is an empty list”, for maximum clarity.

[BreamoreBoy]

This shouldn't be languishing, work has been committed against revision 86742 and there's another patch that apparently just needs a little tweak.

[berkerpeksag]

The patch is not committed yet.

$ ./python
Python 3.5.0a0 (default:979aaa08c067+, Jun 19 2014, 13:01:36) 
[GCC 4.6.3] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from wsgiref.headers import Headers
>>> h = Headers()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() missing 1 required positional argument: 'headers'

Here's an updated patch that addresses Éric's review (and without cosmetic changes to make review easier).

[BreamoreBoy]

Just for the record clicking on "revision 86742" gives "Specified revision r86742 not found".

[berkerpeksag]

Here's a new patch with a whatsnew entry. David, could you review the patch?

[berkerpeksag]

Patch updated. If there's no objections, I'll commit this patch in the next couple of days.

[bitdancer]

Since we don't need to avoid new language features, using the ternary would be better than using the or. This is exactly the sort of situation the ternary was introduced for: making sure the input really is None before applying the default.

Otherwise the patch looks good.

[python-dev]

New changeset a91f0d4a2381 by Berker Peksag in branch 'default':
Issue bpo-5800: headers parameter of wsgiref.headers.Headers is now optional.
http://hg.python.org/cpython/rev/a91f0d4a2381

[berkerpeksag]

Thanks for the patch and review.