Open merwok opened 13 years ago
splitdoc is a hidden gem in pydoc: it’s a little helper to implement docstring splitting as documented in the docstrings PEPs. It is not a one-liner, so I think there is value in making it public in the inspect module.
Unassigning. I don't recall what I intended to do to further this along.
I move the pydoc.splitdoc function to the inspect module. Update the documentation. Add a unittest for this new function.
I can provide an other patch for the backward-compatiblity if this function is used by an other module than pydoc.
The patch looks good, but 'splitdoc' needs to remain a valid name for the function in the pydoc namespace. You could just add 'splitdoc = inspect.splitdoc' after the import statements. (The reason it needs to remain valid is for backward compatibility...there may be people who discovered it and have been importing it from pydoc.)
I don't like this idea. inspect module is about introspection, and not about interpreting its results. I'd keep this function in pydoc and document it if there is noticeable demand for it.
Yury and David, please, can you discuss about this point, or just close this ticket if this one is useless.
Thank you
The precedent has already been set by the 'cleandoc' function, I think. This one seems to go right along with that one.
Yury, what's your feedback about this point?
Thanks
David:
The precedent has already been set by the 'cleandoc' function, I think. This one seems to go right along with that one.
What do you think if we keep the function in pydoc module, but document it and make it public? I agree, that there is a precedent of having non-introspection APIs in inspect, but I'd still like to keep it minimal. Having this function in pydoc also makes sense, as it's more about python docstring convention, than introspection (cleandoc() is used by getdoc(), which is why it is in inspect after all)
Well, perhaps inspect needs a get_doc_synopsis method :)
Actually, I'm not sure that should be a smiley.
I don't really have a strong opinion on this myself (say I'm +0 for inspect), so I asked a couple other core devs here at the sprint (Eric Smith and Eric Snow), and they both thought it should be in inspect rather than pydoc. (Eric's Snow reason is that pydoc is not really a user facing library (actually what he said is "it's a mess"), and both thought it was more appropriate for inspect anyway).
I agree with Éric that exposing splidoc publicly in the inspect module is the right thing. inspect already has other similar functions. If it doesn't land in inspect then the only other place that makes real sense to me would be a new module (docstring?). However, that seems like overkill to me.
Furthermore, pydoc doesn't seem like a good place to expose the function (or perhaps any function \<wink>). It isn't a module relating explicitly to docstrings so much as to exposing API documentation. The use of splitdoc there is an implementation detail while splitdoc itself is generally useful. That said, I would still expect splitdoc to be exposed in pydoc for backward compatibility (via "from inspect import splitdoc").
OK, since it's two-and-a-half votes against one, let's do this. I'll do the final review of the patch and commit it.
Added some comments.
The current patch proposes to add inspect.splitdoc(obj), instead of pydoc.splitdoc(doc). The former takes an object, extracts documentation out of it, and returns a tuple. The latter, just splits the passed doc string.
If you want this function in inspect, we need to find a better name for it, or don't make it to receive an object.
On 16 Apr 2014, at 17:24, Yury Selivanov wrote:
Yury Selivanov added the comment:
The current patch proposes to add inspect.splitdoc(obj), instead of pydoc.splitdoc(doc). The former takes an object, extracts documentation out of it, and returns a tuple. The latter, just splits the passed doc string.
If you want this function in inspect, we need to find a better name for it, or don't make it to receive an object.
----------
Python tracker \report@bugs.python.org\ \http://bugs.python.org/issue12916\
In the inspect module, I think all the functions take a object and not a string, it's the reason why I included the code of pydoc.getdoc() into inspect.splitdoc().
One point, the former ( pydoc.splitdoc() ) takes a string, and returns a tuple. it's not the case with the new version.
Yury,
An other point, as you proposed, I will check the version of Python in an unit test.
But is there a good practice? Here is my way to check that:
Example from my patch for the issue with inspect.getfullargspec()
+ getfullargspec = getattr(inspect, 'getfullargspec', None) + if getfullargspec and sys.version_info >= (3, 7): + self.fail("inspect.getfullargspec() is deprecated since 3.5, " + "you must to remove it in 3.7")
Are you agree with that, or there is a good way for this kind of improvement?
Thank you so much,
Stephane
In the inspect module, I think all the functions take a object and not a string, it's the reason why I included the code of pydoc.getdoc() into inspect.splitdoc().
I understand. But you also do inspect.getdoc or inspect.getcomments, which I don't really like. What's the point of having getcomments there? Are comments considered docstrings? If not, then why is the method called splitdocs?
Don't get me wrong, I'm not trying to pushback on the idea (since everybody is agreeing to have it), I just want the naming and behaviour be consistent.
Are you agree with that, or there is a good way for this kind of improvement?
Having a unittest to check if a deprecated functionality is removed in the future versions was Brett's idea, and I like it. So I think it's good to do the same here. Your way of doing this is fine.
Totally agree with you,
I want to learn how to contribute to cpython and there is a learning curve and it's normal.
So, if you think we need to change the names or the signature of the function, I can work on this issue.
I'd keep the name ("splitdoc"), and let it receive a string. But let's hear what Eric & David think about it.
Ok, I will work on this bug after the feedback of Eric and David.
Stéphane Wirtel - http://wirtel.be - @matrixise
I'd keep the name ("splitdoc"), and let it receive a string.
Yes please.
And it takes a string or an object?
It should receive a string. This is parallel to cleandoc, and I think splitdoc should go in the documentation right after cleandoc.
I will fix this issue asap, but I was too tired with the travel to Belgium.
Hope to propose patch during this week.
Hi all,
Here is a new version of the patch, please, keep me informed and I think I have to modify some parts, but give me your feedback.
Thanks
Hi all,
No news about this issue,
Do you have time for a feedback?
Thanks
There's a small typo in your patch, strign instead of string. Otherwise, looks good to me.
Although it is not documented, inspect.getdoc() may return None instead of a documentation string. In patch 2, inspect.splitdoc() only accepts a string; perhaps it should also accept None? Otherwise you might have to use it like this:
[summary, body] = splitdoc(getdoc(api) or "")
I left a couple of comments on Rietveld.
Hi all,
Here is the last version of this patch for a review, the tests are ok.
Thank you in advance for the time.
Stephane
@berker.peksag Could you review the last patch? and keep me informed?
Thanks,
Here is a 4th patch that allows None as input. Other changes:
I left the pydoc test there, though I don’t see a big need for this test. It is no problem if the deprecated function remains in version 3.7.
Oops, seems I forgot to refresh my patch
Uploading bpo-12916-splitdoc-5.patch:
Hi Martin, you reused my own patch for this issue?
Yes, this is based on your patch, Stéphane. On top of it I added support for splitdoc(None), and made the other changes in the bullet points.
On 1 Feb 2015, at 22:03, Martin Panter wrote:
Martin Panter added the comment:
Yes, this is based on your patch, Stéphane. On top of it I added support for splitdoc(None), and made the other changes in the bullet points.
Great good news.
Hope these patches will be accepted.
I doubt that inspect is better place for splitdoc() than pydoc. So count my voice against moving splitdoc().
Serhiy, would you be in favour of making it public in the pydoc module, as originally suggested by Yury in \https://bugs.python.org/issue12916#msg216238\, or some other module, or are you saying to reject this completely?
I support Yury.
pydoc doesn't have public API other than its CLI and the help() function. I'd cleanup or even rewrite pydoc before declare anything public in it. On the other hand, there are already functions related to splitdoc() in the inspect module: https://docs.python.org/3/library/inspect.html#retrieving-source-code See also bpo-18956.
There is no rush to make splitdoc() public. We can improve pydoc in 3.5 and 3.6 timeline and then decide what's should be part of the public API.
My opposition against moving splitdoc() to the inspect module is not strict and I don't want to fight for it. In an case two-and-a-half votes are larger than one-and-a-half.
When you move splitdoc(), you should get rid of the use pydoc.splitdoc() in the stdlib and add DeprecatedWarning assertion to the test of pydoc.splitdoc(). The existence of pydoc.splitdoc() test is an argument to keep splitdoc() in the pydoc module.
Berker, I agree. Let's wait till 3.6.
I still don't like having this function in the inspect module, and I still don't understand why it should be there.
Ok, so in this case, you are right to move this issue to the Python 3.6 version, and it's too late for 3.5
Thank you for your help and feedbacks.
Stephane
It's not too late for 3.5. Just there is no consensus.
Am I right that this:
pydoc doesn't have public API other than its CLI and the help() function. […] On the other hand, there are already functions related to splitdoc() in the inspect module […].
There is no rush to make splitdoc() public. We can improve pydoc in 3.5 and 3.6 timeline and then decide what's should be part of the public API.
represents the latest on this issue?
We now have the alpha phase of Python 3.6. Can we get a resolution that makes ‘splitdoc’ public somewhere?
I move this issue to 3.8
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: ```python assignee = None closed_at = None created_at =
labels = ['3.8', 'type-feature', 'library']
title = 'Add inspect.splitdoc'
updated_at =
user = 'https://github.com/merwok'
```
bugs.python.org fields:
```python
activity =
actor = 'matrixise'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation =
creator = 'eric.araujo'
dependencies = []
files = ['34853', '35093', '36880', '37949', '37956']
hgrepos = []
issue_num = 12916
keywords = ['patch']
message_count = 48.0
messages = ['143627', '192925', '216229', '216231', '216238', '216240', '216269', '216368', '216374', '216469', '216470', '216486', '216594', '216597', '216598', '216600', '216601', '216602', '216604', '216605', '216606', '216607', '216608', '216952', '216953', '217524', '218102', '221483', '226859', '227144', '229088', '229997', '235139', '235140', '235163', '235195', '235207', '235224', '237538', '237570', '237571', '237600', '237603', '237665', '237668', '237674', '272832', '311417']
nosy_count = 11.0
nosy_names = ['rhettinger', 'eric.araujo', 'r.david.murray', 'Claudiu.Popa', 'bignose', 'eric.snow', 'berker.peksag', 'martin.panter', 'serhiy.storchaka', 'yselivanov', 'matrixise']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12916'
versions = ['Python 3.8']
```