Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-126019 Fix inspect.getsource for classes created in PyREPL#126032
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Lib/inspect.py Outdated
| # Protect against fetching the wrong source in PyREPL | ||
| if object.__module__ == '__main__': | ||
| raise OSError('source code not available') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmm, I'm worried about this being a breaking change.__module__ now has more precedent over__file__--maybe people could have been relying on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah I share similar concerns. Also why this only affects pyREPL and not any othermain module ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Also why this only affects pyREPL and not any othermain module ?
I think because for other__main__ modules,__main__.__file__ points to the correct source code.
In basic REPL,sys.modules["__main__"] doesn't have a__file__ attribute, so it raisesOSError('source code not available') when inspected.
But with PyREPL,sys.modules["__main__"].__file__ is_pyrepl.__main__, which is where the source code is being pulled from. So we could instead skip loading from__file__ if we detect PyREPL is in use, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Wait, we could just delete__file__ from__main__, right?
diff --git a/Lib/_pyrepl/main.py b/Lib/_pyrepl/main.pyindex a6f824dcc4a..17c55eb9c9f 100644--- a/Lib/_pyrepl/main.py+++ b/Lib/_pyrepl/main.py@@ -35,6 +35,7 @@ def interactive_console(mainmodule=None, quiet=False, pythonstartup=False): import __main__ namespace = __main__.__dict__ namespace.pop("__pyrepl_interactive_console", None)+ namespace.pop("__file__", None) # sys._baserepl() above does this internally, we do it here startup_path = os.getenv("PYTHONSTARTUP")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That fixes the problem no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, I think that would be much less invasive.
I've changed the approach to removing Some changes had to be made to tests that expected Wording of the NEWS entry seems poor to me, would gladly apply any suggested improvements. |
| def test_inspect_getsource(self): | ||
| code = "import inspect\nclass A: pass\n\nprint(inspect.getsource(A))\nexit()\n" | ||
| output, exit_code = self.run_repl(code) | ||
| self.assertEqual(exit_code, 0) | ||
| self.assertIn("OSError('source code not available')", output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think that testing whetherinspect.getsource works on other objects (functions, methods, traceback, frames) is worthwhile, but I'm not sure if it should be done in this PR. So, I'd be fine with doing it in a follow-up
python-cla-botbot commentedApr 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Uh oh!
There was an error while loading.Please reload this page.
inspect.getsourcewould fetch sourcecode from_pyrepl.__main__for classes defined in the REPL when using PyREPL becausesys.modules["__main__"]points to that module.This PR makes
inspect.getsourceraiseOSError('source code not available')for classes defined in the REPL by detecting that.__module__is"__main__". Tests pass with this change, but if you know of a corner case that would be broken by it please say so.