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.

Sign up for GitHub

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

Jump to bottom

gh-115392: Fix doctest reporting incorrect line numbers for decorated functions #115440

Merged
merged 3 commits into from
Feb 14, 2024
Merged

gh-115392: Fix doctest reporting incorrect line numbers for decorated functions #115440

merged 3 commits into from
Feb 14, 2024

Conversation

brianschubert
Copy link
Contributor

@brianschubert brianschubert commented Feb 14, 2024 •
edited by bedevere-app bot
Loading

Fixes line numbers reported by doctest for wrapper functions (such as those returned by functools.wraps).

Currently, doctest.DocTestFinder._find_lineno looks at __code__.co_firstlineno to determine the line number that a function's docstring starts at. This is incorrect for wrapper functions, where the location of the function definition is not the same as the location of the docstring being tested.

Wrapper functions that have been updated with functools.update_wrapper have a __wrapped__ attribute that points to the underlying wrapped function. This PR adds a recursive check for __wrapped__ attributes on function objects to locate the original function that defined the docstring-under-test.

@bedevere-app bedevere-app bot mentioned this pull request Feb 14, 2024
Closed
AlexWaygood
AlexWaygood reviewed Feb 14, 2024
View reviewed changes
Lib/doctest.py Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood approved these changes Feb 14, 2024
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm not sure what would happen if you had a class decorator that used functools.wraps(), but I can't think of a realistic reason why you might do that. So I don't think that's worth worrying about right now.

@AlexWaygood AlexWaygood requested a review from sobolevn February 14, 2024 14:40
sobolevn
sobolevn approved these changes Feb 14, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@AlexWaygood AlexWaygood merged commit bb791c7 into python:main Feb 14, 2024
34 checks passed
@AlexWaygood AlexWaygood added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Feb 14, 2024
@miss-islington-app
Copy link

Thanks @brianschubert for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @brianschubert for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 14, 2024
@brianschubert @miss-islington
pythongh-115392: Fix doctest reporting incorrect line numbers for dec…
59acb43 
…orated functions (pythonGH-115440)

(cherry picked from commit bb791c7)

Co-authored-by: Brian Schubert <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 14, 2024

GH-115458 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 14, 2024
@brianschubert @miss-islington
pythongh-115392: Fix doctest reporting incorrect line numbers for dec…
8e81a9b 
…orated functions (pythonGH-115440)

(cherry picked from commit bb791c7)

Co-authored-by: Brian Schubert <[email protected]>
@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Feb 14, 2024
@bedevere-app
Copy link

bedevere-app bot commented Feb 14, 2024

GH-115459 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Feb 14, 2024
AlexWaygood pushed a commit that referenced this pull request Feb 14, 2024
@miss-islington @brianschubert
[3.11] gh-115392: Fix doctest reporting incorrect line numbers for de…
c4e8ffb 
…corated functions (GH-115440) (#115458)

gh-115392: Fix doctest reporting incorrect line numbers for decorated functions (GH-115440)
(cherry picked from commit bb791c7)

Co-authored-by: Brian Schubert <[email protected]>
AlexWaygood pushed a commit that referenced this pull request Feb 14, 2024
@miss-islington @brianschubert
[3.12] gh-115392: Fix doctest reporting incorrect line numbers for de…
13af281 
…corated functions (GH-115440) (#115459)

gh-115392: Fix doctest reporting incorrect line numbers for decorated functions (GH-115440)
(cherry picked from commit bb791c7)

Co-authored-by: Brian Schubert <[email protected]>
lpsinger added a commit to lpsinger/pytest-doctestplus that referenced this pull request Apr 9, 2024
@lpsinger
TST: add tests for ufuncs without docstrings, wrapped or not
0ce2e33 
This is known to fail on Python 3.11.9 because `--doctest-ufunc`
was broken for wrapped ufuncs without docstrings by
python/cpython#115440.
@lpsinger lpsinger mentioned this pull request Apr 9, 2024
Merged
@lpsinger
Copy link

lpsinger commented Apr 9, 2024 •
edited
Loading

This broke doctest discovery for functions that are wrapped but do not define a __doc__. See scientific-python/pytest-doctestplus#248.

@AlexWaygood
Copy link
Member

This broke doctest discovery for functions that are wrapped but do not define a __doc__. See scientific-python/pytest-doctestplus#248.

Would you be able to open a new issue with a minimal repro?

@lpsinger lpsinger mentioned this pull request Apr 9, 2024
Closed
@lpsinger
Copy link

lpsinger commented Apr 9, 2024

This broke doctest discovery for functions that are wrapped but do not define a __doc__. See scientific-python/pytest-doctestplus#248.

Would you be able to open a new issue with a minimal repro?

#117692

@sobolevn sobolevn mentioned this pull request Apr 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@sobolevn sobolevn sobolevn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@brianschubert @lpsinger @AlexWaygood @sobolevn