classification
Title: Add docstrings to fields of urllib.parse results
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: berker.peksag, curioswati, martin.panter, orsenthil, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-12-08 09:15 by serhiy.storchaka, last changed 2016-01-15 05:29 by curioswati. This issue is now closed.

Files
File name Uploaded Description Edit
iss_25822.patch curioswati, 2015-12-11 05:40 Fix for named tuple docstring. review
iss_25822_2.patch curioswati, 2015-12-12 03:18 Modified from previous review. review
iss_25822_3.patch curioswati, 2015-12-14 03:28 review
iss_25822_4.patch curioswati, 2015-12-31 03:56 Added docstring for DefragResult. review
Messages (10)
msg256105 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-08 09:15
Results of urlsplit() and urlparse() functions in the urllib.parse module are named tuple with a number of fields. Since property docstrings are writable now (issue24064), we can provide docstrings for all these fields. See also issue24878.
msg256186 - (view) Author: Swati Jaiswal (curioswati) * Date: 2015-12-11 03:14
I can help with this. Should I propose the changes I am going to make before making them or just create the patch?
msg256197 - (view) Author: Swati Jaiswal (curioswati) * Date: 2015-12-11 05:40
Here is the patch, please review it. Do I need to write any test?
msg256241 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-12-11 22:24
Thanks, Swati. I left a few comments on Rietveld: http://bugs.python.org/review/25822/

A test wouldn't hurt, but you can wait for further review comments to avoid updating tests each time you get a comment.
msg256259 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-12 04:59
[padding]
I left some comments.

I wonder if the doc strings are too specific when they mention “request”, “file”, “download”, “page”, etc. Maybe these could just be examples of what the fields are used for (e.g. “The hierarchical path, such as the path to a file to download”). Or maybe they could be changed to general terms, but this may be hard.

Also, some of them seem redundant. Does it really add anything to say the “query” field is “the query parameter”?
msg256364 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-12-14 03:40
I wouldn't normally write a test for a docstring patch.
msg257237 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-12-30 19:49
Could you please also add docstrings for _DefragResultBase?

I agree that there is no need to write tests.
msg258186 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-01-14 08:11
New changeset 7bfcb8b75ad9 by Senthil Kumaran in branch 'default':
Issue #25822: Add docstrings to the fields of urllib.parse results.
https://hg.python.org/cpython/rev/7bfcb8b75ad9
msg258187 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-01-14 08:17
Thank you, Swati. 

I reviewed and committed your patch. I had to make some minor formatting changes while keeping an eye for the maintainability of the module and doc strings.

* Moved all the doc strings up to a single place just below the namespaces are declared.
* Followed pep-0257 conventions for doc strings.
* Tried DRY for parse result's sub-components.

Thanks for addressing the review comments from other developers.
msg258267 - (view) Author: Swati Jaiswal (curioswati) * Date: 2016-01-15 05:29
Thank you Senthil for the improvements, I'll try to make it better next time :)
History
Date User Action Args
2016-01-15 05:29:35curioswatisetmessages: + msg258267
2016-01-14 08:17:37orsenthilsetstatus: open -> closed
resolution: fixed
messages: + msg258187
2016-01-14 08:11:49python-devsetnosy: + python-dev
messages: + msg258186
2016-01-14 04:42:22orsenthilsetassignee: orsenthil
2015-12-31 03:56:29curioswatisetfiles: + iss_25822_4.patch
2015-12-30 19:49:42serhiy.storchakasetmessages: + msg257237
2015-12-14 03:40:28rhettingersetmessages: + msg256364
2015-12-14 03:28:15curioswatisetfiles: + iss_25822_3.patch
2015-12-12 04:59:21martin.pantersetmessages: + msg256259
2015-12-12 03:18:06curioswatisetfiles: + iss_25822_2.patch
2015-12-11 22:24:38berker.peksagsetnosy: + berker.peksag
messages: + msg256241
2015-12-11 06:45:11serhiy.storchakasetnosy: + orsenthil, martin.panter

stage: needs patch -> patch review
2015-12-11 05:40:37curioswatisetfiles: + iss_25822.patch
keywords: + patch
messages: + msg256197
2015-12-11 03:14:37curioswatisetnosy: + curioswati
messages: + msg256186
2015-12-08 09:15:40serhiy.storchakacreate