Skip to content

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.

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

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

gh-126019 Fix inspect.getsource for classes created in PyREPL #126032

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9b1369e
Fix inspect.getsource for classes created in PyREPL.
devdanzin Oct 27, 2024
5eafc69
Fix test_inspect_getsource.
devdanzin Oct 27, 2024
cc16cac
Add test_class_defined_in_pyrepl to test_inspect.TestBuggyCases.
devdanzin Oct 27, 2024
c76fb0e
📜🤖 Added by blurb_it.
blurb-it[bot] Oct 27, 2024
b3b9cb6
Merge branch 'main' into fix_repl_inspect_getsource
devdanzin Oct 28, 2024
46b88d6
Undo changes to inspect.
devdanzin Oct 28, 2024
e3e82b3
Remove __file__ attribute from __main__ when PyREPL starts.
devdanzin Oct 28, 2024
cc19746
Fix test_inspect_keeps_globals_from_inspected_module.
devdanzin Oct 28, 2024
73d66c1
Update NEWS entry for PR.
devdanzin Oct 28, 2024
2c09d05
Fix NEWS entry for PR.
devdanzin Oct 28, 2024
7448a6f
Remove outdated comment.
devdanzin Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Lib/inspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,11 +818,12 @@ def getfile(object):
raise TypeError('{!r} is a built-in module'.format(object))
if isclass(object):
if hasattr(object, '__module__'):
# Protect against fetching the wrong source in PyREPL
if object.__module__ == '__main__':
raise OSError('source code not available')
Copy link
Member

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?

Copy link
Member

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 other main module ?

Copy link
Member Author

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 other main 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 raises OSError('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?

Copy link
Member Author

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.py
index 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")

Copy link
Member

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?

Copy link
Member

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.

module = sys.modules.get(object.__module__)
if getattr(module, '__file__', None):
return module.__file__
if object.__module__ == '__main__':
raise OSError('source code not available')
raise TypeError('{!r} is a built-in class'.format(object))
if ismethod(object):
object = object.__func__
Expand Down
5 changes: 5 additions & 0 deletions Lib/test/test_inspect/test_inspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,11 @@ def test_class_definition_same_name_diff_methods(self):
self.assertSourceEqual(mod2.cls296, 296, 298)
self.assertSourceEqual(mod2.cls310, 310, 312)

def test_class_defined_in_pyrepl(self):
class A: pass
A.__module__ = "__main__"
self.assertRaises(OSError, inspect.getsource, A)

class TestNoEOL(GetSourceBase):
def setUp(self):
self.tempdir = TESTFN + '_dir'
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_pyrepl/test_pyrepl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,12 @@ def test_null_byte(self):
self.assertEqual(exit_code, 0)
self.assertNotIn("TypeError", output)

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)
Comment on lines +1314 to +1318
Copy link
Member

Choose a reason for hiding this comment

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

I think that testing whether inspect.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


def test_readline_history_file(self):
# skip, if readline module is not available
readline = import_module('readline')
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2024-10-27-11-46-13.gh-issue-126019.I_GHhZ.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix :func:`inspect.getsource` to avoid fetching unrelated source code for classes defined in PyREPL.
Loading