Skip to content

GH-103319: Fix inspect.getsourcelines() to return 1-based line numbers #103226

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

artemmukhin
Copy link
Contributor

@artemmukhin artemmukhin commented Apr 3, 2023

The problem is described in GH-103225.

I did not find any other usages of a line number returned by inspect.getsourcelines() except for pdb. However, I am not sure how many external usages of inspect.getsourcelines() already rely on 0-based line numbers for modules.

@ghost
Copy link

ghost commented Apr 3, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@artemmukhin
Copy link
Contributor Author

@iritkatriel Could you please take a look, as this is a follow-up to #101674 you already reviewed?

Cc @gaogaotiantian

@gaogaotiantian
Copy link
Member

1-based does seem more reasonable. But if we are fixing this, how about the code in inspect.findsource()? It's also 0-based.

@iritkatriel
Copy link
Member

@iritkatriel Could you please take a look, as this is a follow-up to #101674 you already reviewed?

Cc @gaogaotiantian

The bug in pdb that you describe in the issue may be new, but you are changing behaviour of inspect in the PR. Question is whether this is a bug in inspect.

@gaogaotiantian
Copy link
Member

The doc of inspect says this function Return a list of source lines and starting line number for an object. In my opinion, starting line number should not be 0. How is "starting from 0" and "starting from 1" different? They all include line 1 and there's no line 0. The fundamental of 0-based is that [0] means something and that's not the case in line number.

I would think this as a bug, but it's true that this is a breaking behavior. If we kept it as it is in inspect, I'd like to fix it in pdb.

@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Apr 4, 2023
@iritkatriel
Copy link
Member

If there's a fix in pdb that you can do so we don't release a new bug there, then let's do it. Changing inspect probably would require more discussion.

FYI @Yhg1s .

@artemmukhin
Copy link
Contributor Author

@iritkatriel Thank you for the quick response. That sounds reasonable. I can create a separate issue regarding inspect and then relink this PR, leaving #103225 unassigned for now. Is that OK?

@gaogaotiantian Thanks for pointing inspect.findsource() out. I will investigate it further and provide a summary in the new issue.

@artemmukhin
Copy link
Contributor Author

@iritkatriel @gaogaotiantian I have opened a new issue regarding inspect #103319

@artemmukhin artemmukhin changed the title GH-103225: Fix inspect.getsourcelines() to return 1-based line numbers GH-103319: Fix inspect.getsourcelines() to return 1-based line numbers Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review stdlib Python modules in the Lib dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants