Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-17972: Document `findsource` in inspect.rst #6992

Open
wants to merge 1 commit into
base: master
from

Conversation

@Carreau
Copy link
Contributor

Carreau commented May 19, 2018

From the discussion in bpo-1792, it seem that findsource is mostly low
level, while getsource is higher level.

This does not completely fix bpo-17972 as many methonds need to be
marked either private, or documented

https://bugs.python.org/issue17972

From the discussion in bpo-1792, it seem that findsource is mostly low
level, while getsource is higher level.

This does not completely fix bpo-17972 as many methonds need to be
marked either private, or documented
@Carreau Carreau force-pushed the Carreau:findsourcedoc branch from 1e2fb3b to 90b2730 May 21, 2018
Copy link
Member

JulienPalard left a comment

LGTM with nits.

You could maybe also state that findsource does not unwraps?

@@ -511,10 +511,25 @@ Retrieving source code
returned as a single string. An :exc:`OSError` is raised if the source code
cannot be retrieved.

If *object* is a wrapper function (that is to say has a :attr:`__wrapped__`
field, :func:`getsource` will follows the chain of :attr:`__wrapped__`
attributes returning the last object in the chain using :func:`unwrap`

This comment has been minimized.

Copy link
@JulienPalard

JulienPalard Sep 10, 2019

Member

Missing a dot at end of sentence:

Suggested change
attributes returning the last object in the chain using :func:`unwrap`
attributes returning the last object in the chain using :func:`unwrap`.
.. versionchanged:: 3.3
:exc:`OSError` is raised instead of :exc:`IOError`, now an alias of the
former.

.. function:: findsource(object)

Return the entire source file and starting line number for an object.

This comment has been minimized.

Copy link
@JulienPalard

JulienPalard Sep 10, 2019

Member

I'd be explicit that the function is returning a tuple, maybe by starting like this and describing both elements:

Return a tuple ``(lines, lnum)`` where *lines* is ...
Return the entire source file and starting line number for an object.

The argument may be a module, class, method, function, traceback, frame,
or code object. The source code is returned as a list of all the lines

This comment has been minimized.

Copy link
@JulienPalard

JulienPalard Sep 10, 2019

Member

The sentence starting here may be redundent if you fix the previous comment.


The argument may be a module, class, method, function, traceback, frame,
or code object. The source code is returned as a list of all the lines
in the file and the line number indexes a line in that list. An OSError

This comment has been minimized.

Copy link
@JulienPalard

JulienPalard Sep 10, 2019

Member

Please link to the actual exception, at least as a visual hint while reading (this make it rendered differently in HTML):

Suggested change
in the file and the line number indexes a line in that list. An OSError
in the file and the line number indexes a line in that list. An :exc:`OSError`
@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Sep 10, 2019

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.