/usr/local/Cellar/python@3.9/3.9.1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/inspect.py in getfile(object)
664 if getattr(module, '__file__', None):
665 return module.__file__
--> 666 raise TypeError('{!r} is a built-in class'.format(object))
The error is 'X is a built-in class', instead it should be OSError, source code is not available.
Why do you think OSError fits here? Since objects provided on the command line by definition cannot have source code files, the problem isn't that the file is missing or inaccessible. Rather, the value provided to getsource() is wrong. So, in my view, this should rather be ValueError.
Moreover, checking for __main__ is insufficient as there are __main__.py modules after all in many packages.
I agree OSError is not ideal, but I chose it because it's consistent with how inspect reports similar errors. For example, see all instances of OSError, except for first, in this function:
I think it's an implementation detail that class source is not available on interactive prompt, for example function source is available via inspect.getsource().
If a user can do getsource(func) and getsource(cls) in a file, and getsource(func) on interactive prompt, it seems wrong to return a ValueError when the user is trying to getsource(cls) in interactive.
I'm open to change it to ValueError though because I think the err message here is more important to avoid user confusion.
good point about __main__.py files, I didn't remember about them. However they will have the __file__ attribute set, so they will return that right above the check for '__main__' in my patch.
I tested it to make sure:
import inspect
class A: 0
print(inspect.getsource(A))
The unit test was wrong, it was catching the wrong OSError. (I was catching regex first but after some tweaking and changes I lost it and forgot to readd)
The reason it was passing is exactly what you pointed out -- the main.py in unittest package. After I change the class' module to main in the test, it starts looking at sys.modules['main'] which is the one in unittest, which doesn't have this class, which causes the inspect to throw another OSError.
I fixed the unit test by both checking for error regex and patching sys.modules['__main__'] (and of course restoring it later).
Since this is an exception type change, I'd feel more comfortable leaving 3.9 out of the backports here. That way it will be easier for application authors to differentiate between when the change was introduced or not.
ince this is an exception type change, I'd feel more comfortable leaving 3.9 out of the backports here. That way it will be easier for application authors to differentiate between when the change was introduced or not.
New changeset f468ede by Miss Islington (bot) in branch '3.10': bpo-44648: Fix error type in inspect.getsource() in interactive session (GH-27171) (GH-27495) f468ede
I am not sure that OSError was a correct exception. Before this, the only exception raised by inspect.getfile() and inspect.getsourcefile() was TypeError. In particularly this change cause an issue in doctests (#113329).
Even if this change is correct, it should be documented, have versionchanged directives and a What's New entry. But maybe changing it to TypeError is better in long term.
Activity
akulakov commented on Jul 15, 2021
[ins] In [63]: class A:pass
[ins] In [64]: import inspect
[ins] In [65]: inspect.getsource(A)
[snip]
/usr/local/Cellar/python@3.9/3.9.1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/inspect.py in getfile(object)
664 if getattr(module, '__file__', None):
665 return module.__file__
--> 666 raise TypeError('{!r} is a built-in class'.format(object))
The error is 'X is a built-in class', instead it should be OSError, source code is not available.
ambv commented on Jul 16, 2021
Why do you think OSError fits here? Since objects provided on the command line by definition cannot have source code files, the problem isn't that the file is missing or inaccessible. Rather, the value provided to getsource() is wrong. So, in my view, this should rather be ValueError.
Moreover, checking for __main__ is insufficient as there are __main__.py modules after all in many packages.
akulakov commented on Jul 16, 2021
Łukasz:
Thanks for looking at this!
cpython/Lib/inspect.py
Line 930 in 7915c96
I think it's an implementation detail that class source is not available on interactive prompt, for example function source is available via inspect.getsource().
If a user can do getsource(func) and getsource(cls) in a file, and getsource(func) on interactive prompt, it seems wrong to return a ValueError when the user is trying to getsource(cls) in interactive.
I'm open to change it to ValueError though because I think the err message here is more important to avoid user confusion.
I tested it to make sure:
import inspect
class A: 0
print(inspect.getsource(A))
./python.exe ~/temp/main.py
class A: 0
akulakov commented on Jul 16, 2021
I'm not sure though how is the unit test succeeding since the test module should have __file__ set. Looking into it..
akulakov commented on Jul 16, 2021
This is kind of interesting:
The unit test was wrong, it was catching the wrong OSError. (I was catching regex first but after some tweaking and changes I lost it and forgot to readd)
The reason it was passing is exactly what you pointed out -- the main.py in unittest package. After I change the class' module to main in the test, it starts looking at sys.modules['main'] which is the one in unittest, which doesn't have this class, which causes the inspect to throw another OSError.
I fixed the unit test by both checking for error regex and patching sys.modules['__main__'] (and of course restoring it later).
Thanks for helping to catch this :-)
ambv commented on Jul 30, 2021
New changeset 48a6255 by andrei kulakov in branch 'main':
bpo-44648: Fix error type in inspect.getsource() in interactive session (GH-27171)
48a6255
ambv commented on Jul 30, 2021
Since this is an exception type change, I'd feel more comfortable leaving 3.9 out of the backports here. That way it will be easier for application authors to differentiate between when the change was introduced or not.
othmaneelbouri1 commented on Jul 30, 2021
ince this is an exception type change, I'd feel more comfortable leaving 3.9 out of the backports here. That way it will be easier for application authors to differentiate between when the change was introduced or not.
ambv commented on Jul 30, 2021
New changeset f468ede by Miss Islington (bot) in branch '3.10':
bpo-44648: Fix error type in inspect.getsource() in interactive session (GH-27171) (GH-27495)
f468ede
ambv commented on Jul 30, 2021
Thanks, Andrei! ✨ 🍰 ✨
pablogsal commented on Aug 2, 2021
Unfortunately, this PR has broken test_inspect when executed with -R, see:
https://bugs.python.org/issue44808
serhiy-storchaka commented on Dec 22, 2023
I am not sure that OSError was a correct exception. Before this, the only exception raised by inspect.getfile() and inspect.getsourcefile() was TypeError. In particularly this change cause an issue in doctests (#113329).
Even if this change is correct, it should be documented, have versionchanged directives and a What's New entry. But maybe changing it to TypeError is better in long term.