classification
Title: Several small errors in winreg documentation
Type: behavior Stage: resolved
Components: Documentation, Windows Versions: Python 3.4, Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brian.curtin Nosy List: asvetlov, brian.curtin, docs@python, python-dev, stutzbach, zach.ware
Priority: normal Keywords: patch

Created on 2012-10-11 16:12 by zach.ware, last changed 2012-10-31 17:31 by asvetlov. This issue is now closed.

Files
File name Uploaded Description Edit
winreg_example.patch zach.ware, 2012-10-11 16:12 Example of proposed changes (3.2 branch) review
winreg_3.2.patch zach.ware, 2012-10-11 18:41 Patch against 3.2 winreg.c and winreg.rst review
winreg_3.3+.patch zach.ware, 2012-10-30 19:24 Patch against 3.3 winreg.c and winreg.rst review
winreg_3.3+v2.patch asvetlov, 2012-10-31 15:10 review
winreg_3.3+v3.patch asvetlov, 2012-10-31 16:37 review
Messages (18)
msg172655 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-11 16:12
There are several small inconsistencies in the winreg module documentation (docstrings and winreg.rst).  Mostly these are discrepancies between "res" and "reserved", "sam" and "access", and whether or not those two are keyword arguments, but there are some other missing pieces as well.

I'm happy to provide a patch to correct everything, but I'd like a little guidance beforehand.  I have no experience with docstrings in C code, so I've attached a patch that attempts to correct just the CreateKeyEx() docstring.  If it turns out to be ok, I'll go ahead and create a patch fixing the other ones.  I'm comfortable enough with ReST changes that I'll just include those with the final docstring patch.

Also, some of the changes are specific to 3.3+ (due to the WindowsError => OSError change).  Should I provide one patch against default, two against 3.2 and 3.3 (including the 3.2 changes), or two against 3.2 and 3.3 (after a local commit of the 3.2 changes)?  Any of the above is perfectly doable :)

Thanks,

Zach
msg172657 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-10-11 16:21
The patch looks alright, but I would remove the lining up of definitions.
It's probably easiest to do a patch against 3.2 and I can handle the porting on commit.
msg172661 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-11 16:38
Okie doke.  I'll try to have a patch ready for review later this afternoon.
msg172672 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-11 18:41
Here's the patch against 3.2, hopefully I caught everything I meant to :)

Unfortunately, I can't build Python or the docs on this machine, so I can't guarantee I didn't break anything.  I tried to be careful though, especially with changing linebreaks in the docstrings.

The changes for 3.3+ that don't apply to 3.2 are:
- in PC\winreg.c
  - Change all occurrences of "a WindowsError" to "an OSError"
- in Doc\library\winreg.rst
  - Change "a" to "an" in all occurrences of "a :exc:`OSError`"
  - Consolidate all the ..versionchanged:: 3.3 notes into one at the top (like in Doc\library\msvcrt.rst)

Also, I did make a couple of non-cosmetic informational changes other than on 'res' and 'sam', such as listing the correct default 'access' parameter on a couple of functions (judging by my reading of said functions).

Thanks,

Zach
msg172994 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-15 18:34
It occurs to me that I should have asked; should the documentation be changed to match the code, or the code to match the documentation (with regards to the default argument to the access parameter in a few functions)?
msg172995 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-10-15 18:39
Docs should match code. If we did it the other way around we'd probably break something.

Thanks for looking into this. I've been busy the last few days but I will get to the review and application of the patch very soon.
msg172997 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-15 19:20
That's what I figured, so that's what I did in the patch; but I've also seen cases in Python where prior documentation has dictated how the code should work.  Thanks for the confirmation.
msg174162 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-29 23:21
New changeset b9e14b89199b by Brian Curtin in branch '3.2':
Fix #16197. Update docstrings and documentation to match winreg code.
http://hg.python.org/cpython/rev/b9e14b89199b
msg174163 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-10-29 23:22
Pushed fixes for 3.2+

Thanks for the patch!
msg174218 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-30 19:24
Thanks for the commit! 3.2 looks good now.

3.3 and default still need a little work, though; the docstrings still say "WindowsError" instead of "OSError" and the docs say "a OSError" instead of "an OSError".

The attached patch cleans up both issues, as well as consolidating all the versionchanged notices about WindowsError == OSError.
msg174271 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-31 11:15
Not sure consolidating is good idea, ok with other changes.
msg174279 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-31 14:12
The thought on consolidating is to match Doc/library/msvcrt.rst which does the same thing.  Also, when I started reading this page shortly before opening this issue, I was reading most of the page at once and was frankly pretty annoyed by seeing the same notice over and over.  Although I could see the point of having a much shorter blurb on each affected function in addition to an explanatory note at the top.  So instead of

'''
   .. versionchanged:: 3.3
      This function used to raise a :exc:`WindowsError`, which is now an
      alias of :exc:`OSError`.
'''

on each one, something like

'''
   .. versionchanged:: 3.3
      :exc:`WindowsError` is :exc:`OSError`
'''

perhaps?
msg174289 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-31 15:10
What's about compromise from attached file?
msg174291 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-31 15:25
Not bad, but with that scheme we could even go as far as cutting out the 'WindowsError is OSError' bit and make it just 

"""
   ..versionchanged:: 3.3
     See :ref:`above <exception-changed>`.
"""
msg174322 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-31 16:37
I have updated patch.
msg174335 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2012-10-31 17:15
v3 looks fine to me!
msg174337 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-31 17:30
New changeset 2e7da832219d by Andrew Svetlov in branch '3.3':
Issue #16197: Fix several small errors in winreg documentation.
http://hg.python.org/cpython/rev/2e7da832219d

New changeset f1310219c702 by Andrew Svetlov in branch 'default':
Merge issue #16197: Fix several small errors in winreg documentation.
http://hg.python.org/cpython/rev/f1310219c702
msg174338 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-31 17:31
Committed. Thanks, Zachary!
History
Date User Action Args
2012-10-31 17:31:41asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg174338
2012-10-31 17:30:10python-devsetmessages: + msg174337
2012-10-31 17:15:08zach.waresetmessages: + msg174335
2012-10-31 16:37:23asvetlovsetfiles: + winreg_3.3+v3.patch

messages: + msg174322
2012-10-31 15:25:20zach.waresetmessages: + msg174291
2012-10-31 15:10:16asvetlovsetfiles: + winreg_3.3+v2.patch

messages: + msg174289
2012-10-31 14:12:34zach.waresetmessages: + msg174279
2012-10-31 11:15:47asvetlovsetnosy: + asvetlov
messages: + msg174271
2012-10-30 19:24:57zach.waresetstatus: closed -> open
files: + winreg_3.3+.patch
resolution: fixed -> (no value)
messages: + msg174218
2012-10-29 23:22:52brian.curtinsetstatus: open -> closed
resolution: fixed
messages: + msg174163

stage: resolved
2012-10-29 23:21:09python-devsetnosy: + python-dev
messages: + msg174162
2012-10-15 19:20:03zach.waresetmessages: + msg172997
2012-10-15 18:39:14brian.curtinsetmessages: + msg172995
2012-10-15 18:34:48zach.waresetmessages: + msg172994
2012-10-11 18:41:58zach.waresetfiles: + winreg_3.2.patch

messages: + msg172672
2012-10-11 16:38:01zach.waresetmessages: + msg172661
2012-10-11 16:21:15brian.curtinsetassignee: docs@python -> brian.curtin
type: behavior
messages: + msg172657
components: + Windows
2012-10-11 16:12:11zach.warecreate