classification
Title: pkgutil docs should reference glossary terms not PEP 302
Type: enhancement Stage: commit review
Components: Documentation Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Oren Milman, docs@python, eric.araujo, jaysinh.shukla, kushal.das, martin.panter, ncoghlan, orsenthil, python-dev
Priority: normal Keywords: easy, patch

Created on 2014-03-03 11:33 by ncoghlan, last changed 2016-09-07 07:53 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue20842_v1.diff jaysinh.shukla, 2016-06-23 17:03 review
issue20842_v2.diff jaysinh.shukla, 2016-07-14 16:49 review
issue20842-v3.patch orsenthil, 2016-09-05 23:14 review
issue20842-3.3-v5.patch orsenthil, 2016-09-06 00:18
Messages (15)
msg212630 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-03-03 11:33
The pkgutil docs still point at PEP 302 when mentioning loaders, importers, etc. They should reference the glossary terms (:term:`loader`, etc) instead.
msg213382 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2014-03-13 10:32
Will submit a patch for this.
msg269126 - (view) Author: Jaysinh shukla (jaysinh.shukla) * Date: 2016-06-23 17:03
Submitting the patch for this issue. I observed the last message of respected Kushal but because the message was posted too earlier decided to submit patch. Thanks!
msg269151 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-24 02:43
See also Issue 26896. Apparently some of the terms you are linking are used wrongly.
msg269227 - (view) Author: Jaysinh shukla (jaysinh.shukla) * Date: 2016-06-25 10:52
Dear Martin,
    I observed your comment. I can see, issue mentioned by you http://bugs.python.org/issue26896 contains patch of clarifying the reference of Importer, Finder. The issue-26896 is still under review phase. It will be good step to update doc in this issue according to http://bugs.python.org/file42666/issue.diff patch?

Thanks!
msg269235 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-25 12:41
I’m not sure what the best solution is. I am just pointing out the problem. It seems an “importer” as mentioned in PEP 302 is more general than the current glossary definition. E.g. ImpImporter() returns an object that has a find_module() method (which partially makes it both a “meta path finder” and a “path entry finder”), but it has no load_module() method. So it fails the “importer” definition according to the current glossary.
msg270005 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-07-08 19:18
Note that http://bugs.python.org/issue26896 is now closed (the patches proposed in it (with some minor changes) were committed).
msg270424 - (view) Author: Jaysinh shukla (jaysinh.shukla) * Date: 2016-07-14 16:49
Adding updated patch after merging Issue 26896. Requesting to review. Thanks!
msg270551 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-07-16 09:50
I still think the links are too dense. Three links to the same term in two short paragraphs is too much. Do you think it would be okay to just link the first occurrence for pkgutil.ImpImporter?

Also, there is still a problem at least the definition of “finder” used by the ImpImporter documentation. In PEP 302, “a finder object has a single method: finder.find_module(fullname, path=None)”. This seems to be a pretty good description of the ImpImporter API. However, by dropping the reference to PEP 302, and linking to the glossary definition, we are suggesting that ImpImporter implements importlib.abc.MetaPathFinder and/or importlib.abc.PathEntryFinder. Both these APIs require some sort of find_spec() method, which is not provided by ImpImporter.

As I see it, either Python’s “finder” glossary entry should be adjusted to accommodate PEP 302, or we should retain the PEP 302 reference and not point at the wrong definition of “finder”.
msg272150 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-08-08 08:31
ImpImporter was first added in changeset 37808 (https://hg.python.org/cpython/rev/ccc0b5412799) and updated a day later in changeset 37821 (https://hg.python.org/cpython/rev/3135648026c4).
Both of these commits were introduced to support PEP 302. 
Since then, ImpImporter remained quite the same, and was never changed to suit later relevant PEPs (e.g. PEP 420 and PEP 451).

Thus, IMHO, we should go with your second option, Martin, and retain the PEP 302 reference in ImpImporter documentation.

With regard to the accuracy of Python's 'finder' glossary entry, it was updated recently, in changeset 99434 (https://hg.python.org/cpython/rev/88cee7d16ccb), to suit PEP 302, PEP 420 and PEP 451, so ISTM there is no need to adjust it.
msg274464 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-09-05 23:14
I agree with the comment made by Oren. Not all instances of finder should be referenced as generic finder. Some classes and functions are are still PEP 302 specific finder.

In the commit, 3987667bf98f Nick Coghlan modified the following functions to to use importlib module, thus serving the PEP 302 bind. So these can refer to glossary entry for finder. The documentation has been updated with version changed information.

* get_importer (by the underlying change of using only sys.path_importer_cache and removing the use of ImpImporter)
* iter_importers (by using importlib.import_module)

The glossary entry for both finder and loader has been updated to include both PEP 302 finder, and with PEP 420 and PEP 451.

With these changes already in place, I updated the patch to reflect the current state. 

I am proceed with the commit as this is a non-controversial change. If there are comments, please share and I will address them.
msg274474 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-09-06 00:09
For 3.5 version of patch, we have bring in some additional changes from issue26896 which properly clarified finder from importer. These changes are not required in the default branch.
msg274477 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-09-06 00:18
This is 3.3 version of patch attached.
msg274478 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-09-06 00:20
Change committed.

- 3.5 * 103098:ecbad01262c8
- default * 103099:ee58ece83391
msg274775 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-09-07 07:53
New changeset a4360b1b45a8 by Senthil Kumaran in branch 'default':
issue20842 - null merge with 3.5
https://hg.python.org/cpython/rev/a4360b1b45a8
History
Date User Action Args
2016-09-07 07:53:45python-devsetnosy: + python-dev
messages: + msg274775
2016-09-06 00:20:27orsenthilsetstatus: open -> closed
resolution: fixed
messages: + msg274478

stage: patch review -> commit review
2016-09-06 00:18:43orsenthilsetfiles: + issue20842-3.3-v5.patch

messages: + msg274477
2016-09-06 00:09:54orsenthilsetmessages: + msg274474
2016-09-05 23:14:56orsenthilsetfiles: + issue20842-v3.patch
nosy: + orsenthil
messages: + msg274464

2016-08-08 08:31:52Oren Milmansetmessages: + msg272150
2016-07-16 09:50:49martin.pantersetmessages: + msg270551
2016-07-14 16:49:36jaysinh.shuklasetfiles: + issue20842_v2.diff

messages: + msg270424
2016-07-08 19:18:04Oren Milmansetnosy: + Oren Milman
messages: + msg270005
2016-06-25 12:41:51martin.pantersetmessages: + msg269235
stage: needs patch -> patch review
2016-06-25 10:52:02jaysinh.shuklasetmessages: + msg269227
2016-06-24 02:43:20martin.pantersetnosy: + martin.panter
messages: + msg269151
2016-06-23 17:03:02jaysinh.shuklasetfiles: + issue20842_v1.diff

nosy: + jaysinh.shukla
messages: + msg269126

keywords: + patch
2016-06-08 10:36:51berker.peksagsetkeywords: + easy
versions: + Python 3.6, - Python 3.4
2014-03-13 10:32:50kushal.dassetnosy: + kushal.das
messages: + msg213382
2014-03-13 09:28:24eric.araujosetnosy: + eric.araujo
2014-03-03 11:33:20ncoghlancreate