Displaying the source code in tracebacks for Cython-compiled extension modules in IPython no longer works due to PEP 302. Various fixes are possible, the two most obvious ones are:

1. linecache should continue searching for the source file even if loader.get_source() returns None.

2. the method ExtensionFileLoader.get_source() should be removed (instead of being implemented and returning None).

Now why was this broken and how do the above fix that?

When IPython needs to display a traceback, it uses linecache.getlines() to get the source code to display. For Cython extensions, the filename is a correct *relative* filename (it must be a relative filename since Cython does not know where the sources will be after installation).

Since the filename is relative, linecache does not immediately find it, so it looks for a PEP 302 loader. For extension modules (Cython or not), it finds an instance of ExtensionFileLoader. If the loader has a get_source() method, then it uses that to get the sources. Since ExtensionFileLoader.get_source() returns None, linecache stops looking further for sources.

Instead, what should happen is that linecache continues looking for the sources in sys.path where it has a chance of finding them (if they are installed somewhere in sys.path).

On Feb 8, 2018 12:55, "Jeroen Demeyer" <report@bugs.python.org> wrote:

New submission from Jeroen Demeyer <jdemeyer@cage.ugent.be>:

Displaying the source code in tracebacks for Cython-compiled extension
modules in IPython no longer works due to PEP 302. Various fixes are
possible, the two most obvious ones are:

To be clear this is nothing to do with PEP 302, but has more to do with
details of the import system reimplementation of Python 3.3, which PEP 302
predates by quite a lot.

 linecache should continue searching for the source file even if
loader.get_source() returns None.

I don't necessarily agree here. For some modules there may not be a real
file associated with it in the first place, much less on sys.path. I'm not
exactly sure why that fallback is there in the first place, but if a module
does have a __loader__ that should have the say of where the module's
source code is found (if at all).

2. the method ExtensionFileLoader.get_source() should be removed (instead
of being implemented and returning None).

Why? What would that help with? PEP 302 says get_source() can return None
of no sources are found. That ExtensionFileLoader does this is not wrong
(though it might be nice if it had a way to show C sources). It certainly
doesn't know anything about Cython.

If anything, as you and I discussed, Cython should be providing its own
loader for Cython modules (and perhaps have a way to better distinguish
Cython modules from other extension modules).

Now why was this broken and how do the above fix that?

When IPython needs to display a traceback, it uses linecache.getlines() to
get the source code to display. For Cython extensions, the filename is a
correct *relative* filename (it must be a relative filename since Cython
does not know where the sources will be after installation).

Since the filename is relative, linecache does not immediately find it, so
it looks for a PEP 302 loader. For extension modules (Cython or not), it
finds an instance of ExtensionFileLoader. If the loader has a get_source()
method, then it uses that to get the sources. Since
ExtensionFileLoader.get_source()
returns None, linecache stops looking further for sources.

Instead, what should happen is that linecache continues looking for the
sources in sys.path where it has a chance of finding them (if they are
installed somewhere in sys.path).

The problem with this analysis is that the fact that this used to work at
all was relying on undocumented behavior. Also in the case of Sage, which
was using this to find Cython sources, it's because we were putting Cython
sources on a sys.path entry which is not a normal thing to do.

I don't think there's a bug in Python here, and that this is a problem that
needs to be solved on the Cython end.

> Why? What would that help with? PEP 302 says get_source() can return None [if] no sources are found.

Returning None implies that it's absolutely impossible that there are sources to be found. But in certain cases (in the case of Cython), extension modules do have sources. So ExtensionFileLoader should assume that there may or may not be sources to be found. That is signalled by not implementing "get_source()".

> I don't think there's a bug in Python here, and that this is a problem that needs to be solved on the Cython end.

I'm not necessarily disagreeing here.

It all depends on how ExtensionFileLoader is meant to be used. Should it try to support extension modules in the narrow sense (hand-written .c files) or in the broad sense (any kind of extension module, possibly auto-generated).

Doing that properly in Cython would almost certainly need PEP 489 support though (Cython is in the process of implementing that, but apparently it's not easy)

Alternatively, instead of not implementing `loader.get_source()`, we could define a new semantic: if it returns `NotImplemented`, assume that the loader doesn't know how to get the sources. In that case, linecache would fall back to the old behaviour.

+1, that seems obvious to me like better behavior.

As I noted on python-ideas, continuing to search along sys.path if the loader returns None seems to match what the linecache docs say. If the issue had been phrased as "the implementation of linecache doesn't follow the docs" then I'd say fine, that's a reasonable report of something that we should fix.

But Erik's points are valid - the fact that the documentation describes a certain behaviour doesn't mean that it can't be wrong - and the fact that no-one else has flagged up the discrepancy before now implies that it's a relatively rare case.

IMO, debating whether a None return means there's absolutely no chance of there being source is silly - the best the loader can say is that it can't provide source. But that doesn't prove anything.

On the other hand, what is Cython source code even doing on sys.path? The whole discussion here seems to be based on the premise that extension modules (machine code) will be shipped with source code that will get installed alongside the binary. That's not the case for C extensions, but appears to be for Cython. I sort of understand the benefit, but it does seem to be a practice peculiar to one scenario, and I'm not sure the standard library should deal with it. (There's a performance cost to that path search, and I'm struggling to see how a Python programmer would benefit from access to the source code of a compiled extension anyway).

So I think that *either* we should fix the docs to be clearer that the path search is only done if the loader is not present or returns None, *or* we should implement the path search when the loader returns None. But I don't see that there's a strong argument here yet for changing the code - after all, it appears that the current behaviour has been round since Python 3.3, so it's not exactly an urgent issue. And unless someone is arguing that the change get backported, Cython still needs to work around the issue for users of older Pythons.

> IMO, debating whether a None return means there's absolutely no chance of there being source is silly - the best the loader can say is that it can't provide source.

I don't think it is silly: if "None" means that the loader cannot provide the source, then it makes sense to continue looking for the source. If "None" means that there is certainly no source, then it does not make sense to continue looking.

> On the other hand, what is Cython source code even doing on sys.path?

It's specifically installed for tracebacks (and other debugging). In a way, it's no different from installing both .py and .pyc files. Nobody questions that, even though the .pyc files are sufficient.

> I sort of understand the benefit, but it does seem to be a practice peculiar to one scenario

Sure, it's particular but it's a real existing scenario.

> and I'm not sure the standard library should deal with it.

Depends what you mean with "deal with it". Basically, all that I'm asking for is at least one officially supported way to enable finding sources for extension modules. There are many possible ways to fix this, that's why I made this post to python-ideas.

> I'm struggling to see how a Python programmer would benefit from access to the source code of a compiled extension anyway.

Cython code looks very much like Python. Very likely, a Python programmer would understand the Cython code. Anyway, this is besides the point: even if it just benefits the author of the Cython code, it's already useful.

> it's not exactly an urgent issue.

I never proposed to make it a release blocker :-)
It's not urgent but it should still be fixed.

> Cython still needs to work around the issue for users of older Pythons.

No. Older versions already work because there is no ExtensionFileLoader. The changes in Python 3.x(?) actually broke this. Certainly in Python 2.7, those tracebacks work fine.

>> IMO, debating whether a None return means there's absolutely no chance of
>> there being source is silly - the best the loader can say is that it can't
>> provide source.
> 
> I don't think it is silly: if "None" means that the loader cannot provide the
> source, then it makes sense to continue looking for the source. If "None"
> means that there is certainly no source, then it does not make sense to
> continue looking.

It's silly to think that the loader can *ever* say that there's "certainly" no
source, so debating whether None means that is silly. The only thing None could
*possibly* mean is that the loader can't give you the source. No more, no less.

>> On the other hand, what is Cython source code even doing on sys.path?
> 
> It's specifically installed for tracebacks (and other debugging). In a way,
> it's no different from installing both .py and .pyc files. Nobody questions
> that, even though the .pyc files are sufficient.

The key difference is that Python source only needs Python to compile it. We
have Python source on sys.path and Python automatically compiles it to bytecode
as needed. You can't put Cython source on sys.path and have the Python
interpreter compile it for you. Basically, sys.path is for code that the Python
interpreter can (compile and) run, and it's at least arguable that non-Python
source doesn't belong on there.

I'm not trying to *make* that argument, though. All *I* am trying to say is
that it's not exactly surprising if stdlib code doesn't expect to find
non-Python source on sys.path.

>> I sort of understand the benefit, but it does seem to be a practice peculiar
>> to one scenario
> 
> Sure, it's particular but it's a real existing scenario.

... that hasn't worked since Python 3.3, but no-one has noticed or complained
until now.

>> and I'm not sure the standard library should deal with it.
> 
> Depends what you mean with "deal with it". Basically, all that I'm asking for
> is at least one officially supported way to enable finding sources for
> extension modules. There are many possible ways to fix this, that's why I
> made this post to python-ideas.

But the standard library has no need to ever find source for extension modules
(unless I'm unaware of some detail of the Cython/IPython specific situation).
So there's no need for the stdlib to be involved - Cython could define a
protocol for finding source code independently, and IPython (and anything else
that wants to display the source of a Cython extension) could use it.

>> I'm struggling to see how a Python programmer would benefit from access to
>> the source code of a compiled extension anyway.
> 
> Cython code looks very much like Python. Very likely, a Python programmer
> would understand the Cython code. Anyway, this is besides the point: even if
> it just benefits the author of the Cython code, it's already useful.

Point taken.

>> it's not exactly an urgent issue.
> 
> I never proposed to make it a release blocker :-) It's not urgent but it
> should still be fixed.

Note that I haven't said it shouldn't be fixed, merely that I'm not as
convinced, having read this discussion, that having linecache do a path search
if the loader returns None is *necessarily* the best solution here. I do still
feel that if we don't make that change, then the linecache docs should be
clarified, but I wouldn't say the linecache docs on the whole module_globals
argument are very clear in the first place - I certainly wouldn't know how to
use that argument based on the current docs.

I'm going to leave the call on whether your proposed change to linecache is OK
to someone who understands (or has the time to review and confirm) the impact
on the 4 places in the stdlib where it's called with a module_global parameter
- asyncio.base_tasks, bdb (twice) and pdb. What I will say is that the code in
the PR looks fine to me as an implementation of the proposed change.

I'm strongly against introducing any extra complexity into the loader
get_source function signature. I think it's fine as it is, and trying to add
nuances to the meaning behind None will only make life harder for people
implementing loaders.

Ideally, of course, there would be a CythonExtensionLoader that handled this in
get_source. But it's hard to see how such a thing would work as "Cython
extensions" don't look any different from extensions built with C, or Rust, or
anything else.

>> Cython still needs to work around the issue for users of older Pythons.
> 
> No. Older versions already work because there is no ExtensionFileLoader. The
> changes in Python 3.x(?) actually broke this. Certainly in Python 2.7, those
> tracebacks work fine.

By "older Pythons" I meant 3.3+. ExtensionFileLoader was introduced in 3.3, so
presumably this issue exists in all versions of Python from 3.3 onwards. I
don't consider 2.7 relevant here, as the import machinery was completely
different then.

Of course, it's up to Cython whether they choose to care about handling this
for Python 3.x users earlier than 3.8 (or earlier, if it's agreed that this
should be backported). And I should note that I'd be less worried about the
potential impact on existing code if this were being proposed solely for 3.8+.
But the way you're talking here, I'm assuming that you're hoping for a
backport. Please correct me if that assumption is wrong.

Since you are asking various "meta" questions, let me explain the wider context first.

This bug report is coming from SageMath. Currently, SageMath only supports Python 2.7 but we are slowly and steadily porting it to Python 3. This bug is one of the many issues that we found while working on this port.

It could very well be that SageMath is the only Cython project which *installs* the source code (although it would make sense to do that for all Cython projects once this bug is fixed). So the fact that nobody has complained before is simply because the only project that could hit this bug is running on Python 2 only.

Also, people may be so used to not seeing Cython code in tracebacks that they don't question it. They may think that it's just a fact of life that it doesn't work.

Concerning backporting to 3.y.z, that is not my decision to make. Of course, I would like to see this backported (it would be trivial to do that), but that should be decided by whoever accepts the pull request. In any case, I don't see why that should influence whether the patch should be accepted for 3.8. That seems to be putting things in the wrong order: first, the patch is applied to 3.8 and then we decide whether to backport it.

> But the standard library has no need to ever find source for extension modules
> So there's no need for the stdlib to be involved

The standard library is not a closed system. It's not meant to support only itself, it's supposed to be an API. If linecache.getlines() is the Python API to get source code, then Cython code should use that API. Having a different competing API for getting source code (for other projects like Cython) is really the worst possible solution: some tools will only use linecache, other tools will use the new API and this will be a mess.

> Note that I haven't said it shouldn't be fixed, merely that I'm not as
> convinced, having read this discussion, that having linecache do a path search
> if the loader returns None is *necessarily* the best solution here.

Do you have other proposals? Like I said, the only thing that I want is one officially supported way to have the loader answer to linecache "I don't know where the sources are but continue looking for them".

> Ideally, of course, there would be a CythonExtensionLoader that handled this in get_source.

That would be ideal solution indeed and it's the first thing that we tried to fix this.

Unfortunately for Cython, PEP 302 (and in particular the get_source signature) was written with the assumption that a *single* module only has a *single* source file. This assumption doesn't hold for Cython code: like C, it supports include/declaration files which can contain code. So my conclusion is that loader.get_source() simply cannot work for Cython.

How does CPython display the source for tracebacks in Cython modules? It seems to work there as long as the Cython .pyx files are somewhere in the import path.

> How does CPython display the source for tracebacks in Cython modules? It seems to work there as long as the Cython .pyx files are somewhere in the import path.

Is that in Python 3.x? The issue we're discussing is only in Python 3.3+

Yes, I tried the Python 3.5 that comes with my system as well as the latest checkout from github. Both show source code lines in tracebacks for me.

I used a rather simple test setup, just two directories with .so and .pyx which I added to sys.path (any order seems to work). I haven't checked a proper install of an extension module via pip/...

That's interesting. It sounds like linecache might be working differently then.

> How does CPython display the source for tracebacks in Cython modules?

Do you mean the "python" command-line program? That uses a different algorithm: to find a file FOO, it searches

[os.path.join(p, os.path.basename(FOO)) for p in sys.path]

This is completely broken because it ignores all leading path components of FOO. So it will only work if your file is in the root of your package, not in a subdirectory.

> Do you mean the "python" command-line program?

Yes, that's what I used.

>  That uses a different algorithm:

I see, it only works for top-level modules. You're right, that's not a real solution.

This problem isn't unique to Cython extension modules - it exists for pyc-only distribution of pure Python files as well.

The underlying problem is that we don't currently have a mechanism comparable to JavaScript source maps, whereby a preprocessed runtime artifact can provide a reference back to a more debugging-friendly representation that's stored somewhere else (whether that's adjacent to the compiled form, in a different directory elsewhere on the same machine, or even on a remote web server).

So I think asking how ExtensionFileLoader.get_source() should behave is likely looking at the problem at the wrong level: a better question may be to ask what feature *linecache* is missing to allow it to correctly report source lines for modules that do *not* include their source code when installed, but do have that source code available in a shadow directory, where the only things that change are the root location (which could potentially even be a HTTP or HTTPS URL), and a fixed transformation on the module filename itself (e.g. replacing "*.so" with "*.c", or "*.pyc" with "*.py").

Given such a feature in Python 3.8, and support for it in Cython, the enhanced linecache module could then be published to PyPI as backports.linecache, providing access to this improved behaviour on all Python 3 versions that SageMath wants to support.

As a completely minimal strawman design that's the closest fit to what SafeMath is already doing: what if there was a "linecache.fallback_source_path" attribute that linecache searched with importlib whenever a loader returned None to indicate that the module existed, but not in a format that provided source code?

Then instead of adding the source directory to sys.path (which was only working because the legacy import system never implemented PEP 302 properly), SageMath could instead add the source directory to that new linecache.fallback_source_path list.

(Given a separate linecache.fallback_metapath, a suitably clever Finder implementation could then even provide the URL source lookup feature)

There'd be some API design issues to work out around ensuring that directories added to linecache.fallback_source_path are added in the same relative order as directories on sys.path, but the end result would likely be more reliable and resilient than relying on "additional directory installed later in sys.path" as the fallback strategy for handling sourceless modules.

> Then instead of adding the source directory to sys.path

What's wrong with that? Installing the .pyx sources together with the .so compiled modules makes a lot of sense to me: it is very analogous to installing the .py sources together with the .pyc byte-compiled files. In https://bugs.python.org/issue32797#msg315965 Paul Moore disagreed with that analogy, but I don't quite understand why.

And if ${prefix}/lib/pythonX.Y/site-packages/PKGNAME is not a good place to store the installed sources, where would you want to install them otherwise?

> (which was only working because the legacy import system never implemented PEP 302 properly)

Not sure what you mean here...

> What's wrong with that? Installing the .pyx sources together with the .so compiled modules makes a lot of sense to me: it is very analogous to installing the .py sources together with the .pyc byte-compiled files. In https://bugs.python.org/issue32797#msg315965 Paul Moore disagreed with that analogy, but I don't quite understand why.

Because if someone deletes the .pyc file, the interpreter can (and will) regenerate it using the .py file. If someone deletes the .so file, you're stuck. Having the .pyx file present won't help you.

In my view (and that of the documentation for sys.path), sys.path is where you put things that the Python interpreter can load - .so files, .pyc files and .py files.

Looking for source to report as part of a traceback is a separate process, and there's no immediate reason why it should involve sys.path. The fact that historically it did (because that worked for pure Python modules that shipped with source) doesn't mean it's the right solution. I'd rather see a well-designed protocol for "find me the source for this module" that tools like Cython can support, than trying to patch up the loader protocol and sys.path to do something they weren't ever designed for.

> In my view (and that of the documentation for sys.path), sys.path is where you put things that the Python interpreter can load - .so files, .pyc files and .py files.

It's quite typical for packages to install stuff in site-packages which the interpreter cannot load. In fact, that's the exactly the point of the package_data argument to setup(), see https://packaging.python.org/guides/distributing-packages-using-setuptools/#package-data

Just as an example, Numpy installs tons of stuff inside site-packages/numpy/ (header files, configuration files, data files for the testsuite)

On Sat, Aug 4, 2018, 11:52 Jeroen Demeyer, <report@bugs.python.org> wrote:

>
> Jeroen Demeyer <J.Demeyer@UGent.be> added the comment:
>
> > In my view (and that of the documentation for sys.path), sys.path is
> where you put things that the Python interpreter can load - .so files, .pyc
> files and .py files.
>
> It's quite typical for packages to install stuff in site-packages which
> the interpreter cannot load. In fact, that's the exactly the point of the
> package_data argument to setup(), see
> https://packaging.python.org/guides/distributing-packages-using-setuptools/#package-data
>
> Just as an example, Numpy installs tons of stuff inside
> site-packages/numpy/ (header files, configuration files, data files for the
> testsuite)
>

I think the key point no one is saying is that .pyc files are no longer on
sys.path because they are in __pycache__ and those aren't importable unless
you access them as namespace packages. So we have been moving to keeping
artifacts like .pyc files off of sys.path.

As Nick said, we have no generalized concept of source maps and I think
coming up with that is what will be required to solve this as i personally
don't view Loader.get_source() is not meant to be a generalized concept of
some form of source code but Python source code.

> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue32797>
> _______________________________________
>

To everybody who mentioned that Cython sources don't belong on sys.path: where *do* they belong then?

One issue which hasn't been mentioned before is packaging. By installing Cython sources along with the generated .so files, we can re-use all the existing tooling from distutils/setuptools/wheel to install those source files. If we want to install them in a different way, ideally it should be done in a way sanctioned by the PyPA and the tools should support it. We should then also change linecache to search for source files in that directory.

Unless somebody can come up with a simple suggestion, this seems to require basically a PEP. I don't plan to write that PEP and fight for it, since this can easily be fixed with essentially a 1-line patch to linecache, as I proposed in https://github.com/python/cpython/pull/6653

On Sat, Aug 4, 2018, 16:07 Jeroen Demeyer, <report@bugs.python.org> wrote:

>
> Jeroen Demeyer <J.Demeyer@UGent.be> added the comment:
>
> To everybody who mentioned that Cython sources don't belong on sys.path:
> where *do* they belong then?
>

In a subdirectory similar to __pycache__.

> One issue which hasn't been mentioned before is packaging. By installing
> Cython sources along with the generated .so files, we can re-use all the
> existing tooling from distutils/setuptools/wheel to install those source
> files.

Technically this doesn't fall under our purview but that of distutils-sig.
Plus how distutils or setuptools do anything is an implementation detail
and it's more important how a wheel would handle this (if any special
support is even necessary). Remember, PEP 517 us our future and is there to
break the monopoly of setuptools.

If we want to install them in a different way, ideally it should be done in
> a way sanctioned by the PyPA and the tools should support it. We should
> then also change linecache to search for source files in that directory.
>

Yep.

> Unless somebody can come up with a simple suggestion, this seems to
> require basically a PEP.

Or at least a new API.

I don't plan to write that PEP and fight for it, since this can easily be
> fixed with essentially a 1-line patch to linecache, as I proposed in
> https://github.com/python/cpython/pull/6653
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue32797>
> _______________________________________
>

Regarding PEP 302 compliance: until Python 3.3, the builtin import system wasn't present on sys.metapath - it was its own implicit legacy thing. The change in Python 3.3 was to fix that, and make it work based on a handful of default sys.metapath entries. Not coincidentally, 3.3 was also the first release where the import system was finally added to the language reference rather than being wholly implementation dependent: https://docs.python.org/3/reference/import.html

I think that also points the way forward for providing a useful import spec compliant fallback for sourceless files in linecache in a way that could be backported to earlier Python 3 versions: it's possible to create a FileFinder [1] instance that only looks for source files, and ignores files that don't provide source code.

Given that, the changes needed to make SageMath tracebacks "just work" the way they currently do in Python 2 would be:

1. Enhance PathFinder [2] to allow specification of which path importer cache and path_hooks entries to use
2. Set up a source-only metapath in linecache with a single default entry: a PathFinder configured with a source-only FileFinder instance as a path hook
3. Fall back to the source-only metapath if the main import system indicates the module is being loaded from a sourceless format.

[1] https://docs.python.org/3/library/importlib.html#importlib.machinery.FileFinder
[2] https://docs.python.org/3/library/importlib.html#importlib.machinery.PathFinder

FWIW, I can see that Cython is a special case because it can control the source line mapping and reporting through the C-API. Other code generators might not be able to do that, e.g. for a DSL or template language that gets translated to Python code, for which an explicit source mapping would be required.

But, I definitely see the analogy between .py files and .pyx files. In fact, .pyx files often originate from renaming .py files. And keep in mind that Cython can also compile .py files, which leads to .so files in the same package directory.

Installing the .pyx files alongside with the .so files is currently easy for our users because distutils does it automatically. Changing the place where they get installed would require tooling support and most likely also changes on user side in all Cython projects out there (unless it's configured through the distutils Extension object, which Cython can control).

The recommended way to structure Cython packages is to have the .pyx files where the resulting extension modules go, in their package, so that both end up next to each other with "setup.py build_ext --inplace", but also to make relative imports and inclusions work through the normal path lookup mechanism in Cython. Keeping two identical package structures in different places, one for .py files and generated .so files, and one for Cython source files (.pyx, .pxd, .pxi), is not desirable from a user perspective, and would require namespace packages for the lookup, where otherwise a simple single Python package structure would suffice. That would complicate Cython's internal compile time import lookup system quite a bit.

If we keep that single package structure (and from a user's perspective, I'm all for it), but change the way source files are looked up, we would have to additionally come up with a way to support the debugging case in a source checkout, where .pyx files and .so file are necessarily in the same package directory, together with the .py files.

This whole idea looks backwards and complicated. As Brett noted, .pyc files were moved out of the source tree, because they are build time artifacts and not sources. With that analogy, it's the .so files that would have to move, not the .pyx or .py source files, which are sources for the compiled .so files.

> In a subdirectory similar to __pycache__.

I thought about that, but I don't like the idea because then the directory structure of the actual sources would be different from the directory structure of the installed sources. So for example, how should "pip install -e" install the sources?

Nick: do you agree that this "source-only metapath" should by default contain an entry to look in sys.path for source files?

> Keeping two identical package structures in different places, one for .py files and generated .so files, and one for Cython source files (.pyx, .pxd, .pxi), is not desirable from a user perspective, and would require namespace packages for the lookup, where otherwise a simple single Python package structure would suffice. That would complicate Cython's internal compile time import lookup system quite a bit.

This is an important point. You shouldn't consider these Cython files as "extra stuff", they really should be considered as part of the package.

On Sun, 5 Aug 2018 at 06:51, Stefan Behnel <report@bugs.python.org> wrote:
> This whole idea looks backwards and complicated. As Brett noted, .pyc files were moved out of the source tree, because they are build time artifacts and not sources. With that analogy, it's the .so files that would have to move, not the .pyx or .py source files, which are sources for the compiled .so files.

I disagree. In *Python* terms:

* .py files are the executable units of Python code
* .so/.pyd files are the executable units of extension modules
* .pyc/.pyo files are runtime artifacts that cache the results of the
compilation step of loading .py code. There's no equivalent for
.so/.pyd files as there's no compilation step needed when loading
them.
* .c, .h., .pyx, ... files are the source code for .so/.pyd files.
There's no equivalent for .py files, because Python code is executable
from source.

Executable units of code go on sys.path. Cached runtime artifacts go
in __pycache__. There's no defined location for source code. Whether
you agree with the above or not, please accept that it's a consistent
view of things. Sourceless distributions of Python code are an oddball
corner case, but they do *not* demonstrate that Python is a compiled
language and the .pyc/.pyo is the "executable" and the .py file is the
"source". At least not in my opinion.

It's entirely reasonable that we want to ensure that an exception
object, whether raised from a Python file or a compiled extension
module, includes a reference to the source of the issue, and that the
traceback mechanism can, if at all possible, locate that source and
display it in the exception traceback. That's basically what the
loader "get_source" method is for - isn't the problem here simply that
get_source for .pyd/.so files returns None, which in turn is because
there's no obvious way to get to the source file for a general binary
(and so the current implementation takes the easy way out and gives
up[1])?

Note that Cython is *not* unique here. Any tool that generates
.so/.pyd files has the same problem, and we should be looking for a
general solution. C code is the obvious example, and while I doubt
anyone is going to ship C sources with a Python extension, it's still
a valid case. Doesn't CFFI have a mode where it compiles a .pyd at
wheel build time? Shouldn't that be able to locate the source? As a
separate example, consider Jython, which may want to be able to locate
the Java (or Groovy, or Scala, ...) source for a .class file on
sys.path, and would have the same problem with get_source that CPython
does with .so/.pyd files.

So basically what's needed here is a modification to
ExtensionFileLoader to return a more useful value from get_source. How
it *does* that is up for debate, and brings in questions around what
standards the packaging community want to establish for shipping
extension sources, etc. And because the resulting decisions will end
up implemented as stdlib code, they need to be properly thought
through, as the timescales to fix design mistakes are quite long.
"Bung stuff on sys.path" isn't really a well thought through solution
in that sense, at least IMO - regardless of the fact that it's in
effect what linecache supported prior to Python 3.3.

So as a way forward, how about this:

1. Agree that what we actually need a better get_source method on
ExtensionFileLoader
2. Adjourn to distutils-sig to agree a standard for where tools that
generate .pyd/.so files should put their sources, if they should
choose to ship their sources in the wheel with the final built
extension.
3. Come back here with the results of that discussion to produce a PR
that implements that change to ExtensionFileLoader

The one possible fly in the ointment is if there are use cases that we
need to support where a single .so/.pyd file is built from *multiple*
source files, as get_source doesn't allow for that. Then it's back to
the drawing board...

Paul

[1] I remember working on the original design of PEP 302 and "take the
easy way out and give up" is *precisely* the thinking behind this :-)

> The one possible fly in the ointment is if there are use cases that we
need to support where a single .so/.pyd file is built from *multiple*
source files, as get_source doesn't allow for that.

Yes, we must support that. A cython module may have multiple sources. The simplest solution to solve that would be a new PEP 302 API, something like get_source_from_filename(self, filename)

So, where is the filename coming from? Looking at exception and frame objects, I don't see a "source filename" attribute anywhere - have I missed something here?

While I'd be inclined to agree with Paul's analysis if CPython were a greenfield project, I also think if SageMath had already been ported to Python 3.2, we would have considered this change a behavioural regression between 3.2 and 3.3 resulting from the importlib migration and worked out a way to get it to work implicitly again.

Now, though, we need a way for SageMath to get it working on releases up to and including 3.7, *without* any help from Cython or CPython, since it needs to work with existing Cython releases on existing CPython versions to avoid presenting itself to SageMath users as regression introduced by switching from Python 2 to Python 3.

To do that, I believe it can be made to work in much the same way it did in Python 2 if SageMath were to do the following:

1. Define a subclass of ExtensionModuleLoader [1] that overrides get_source() to also look for a ".pyx" file adjacent to the extension module. This would also look for any of the file suffixes in SOURCE_SUFFIXES if a .pyx file isn't found.
2. Create an instance of FileFinder [3] that uses the custom loader subclass for any of the file suffixes in EXTENSION_SUFFIXES [4]
3. Replace the regular file finding hook in sys.path_hooks with  the path_hook method from that new FileFinder instance (by default, there's only one FileFinder hook installed, and it can currently be identified as "obj.__name__ == 'path_hook_for_FileFinder')
4. Invalidate importlib's caches, so any future extension module imports will use the custom extension module loader, rather than the default one

[1] https://docs.python.org/3/library/importlib.html#importlib.machinery.ExtensionFileLoader
[2] https://docs.python.org/3/library/importlib.html#importlib.machinery.SOURCE_SUFFIXES
[3] https://docs.python.org/3/library/importlib.html#importlib.machinery.FileFinder
[4] https://docs.python.org/3/library/importlib.html#importlib.machinery.EXTENSION_SUFFIXES

On Sun, Aug 5, 2018, 08:55 Nick Coghlan, <report@bugs.python.org> wrote:

>
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> While I'd be inclined to agree with Paul's analysis if CPython were a
> greenfield project, I also think if SageMath had already been ported to
> Python 3.2, we would have considered this change a behavioural regression
> between 3.2 and 3.3 resulting from the importlib migration and worked out a
> way to get it to work implicitly again.
>

Perhaps, but no point on dwelling on 4 releases ago. 😉

One other thing I will say is that the PEP 302 APIs were seemingly designed
for working with Python source and were not necessarily oriented towards
alternative code representations that were executed, e.g. Quixote was
supported because it transpiled to Python source (although Paul is a
co-author on the PEP and could more definitively answer 😄). IOW the APIs
are oriented towards the Python programming language, not the CPython
interpreter. This is why, for instance, I'm suggesting that if a more
general solution is desired to be made official, then trying to force
everything into the current APIs might not turn out well.

> Now, though, we need a way for SageMath to get it working on releases up
> to and including 3.7, *without* any help from Cython or CPython, since it
> needs to work with existing Cython releases on existing CPython versions to
> avoid presenting itself to SageMath users as regression introduced by
> switching from Python 2 to Python 3.
>
> To do that, I believe it can be made to work in much the same way it did
> in Python 2 if SageMath were to do the following:
>
> 1. Define a subclass of ExtensionModuleLoader [1] that overrides
> get_source() to also look for a ".pyx" file adjacent to the extension
> module. This would also look for any of the file suffixes in
> SOURCE_SUFFIXES if a .pyx file isn't found.
> 2. Create an instance of FileFinder [3] that uses the custom loader
> subclass for any of the file suffixes in EXTENSION_SUFFIXES [4]
> 3. Replace the regular file finding hook in sys.path_hooks with  the
> path_hook method from that new FileFinder instance (by default, there's
> only one FileFinder hook installed, and it can currently be identified as
> "obj.__name__ == 'path_hook_for_FileFinder')
> 4. Invalidate importlib's caches, so any future extension module imports
> will use the custom extension module loader, rather than the default one
>

This sounds like it would work to me and doesn't require any special
changes on our part to function.

-Brett

> [1]
> https://docs.python.org/3/library/importlib.html#importlib.machinery.ExtensionFileLoader
> [2]
> https://docs.python.org/3/library/importlib.html#importlib.machinery.SOURCE_SUFFIXES
> [3]
> https://docs.python.org/3/library/importlib.html#importlib.machinery.FileFinder
> [4]
> https://docs.python.org/3/library/importlib.html#importlib.machinery.EXTENSION_SUFFIXES
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue32797>
> _______________________________________
>

On Sun, 5 Aug 2018 at 18:10, Brett Cannon <report@bugs.python.org> wrote:

> One other thing I will say is that the PEP 302 APIs were seemingly designed
> for working with Python source and were not necessarily oriented towards
> alternative code representations that were executed, e.g. Quixote was
> supported because it transpiled to Python source (although Paul is a
> co-author on the PEP and could more definitively answer 😄). IOW the APIs
> are oriented towards the Python programming language, not the CPython
> interpreter. This is why, for instance, I'm suggesting that if a more
> general solution is desired to be made official, then trying to force
> everything into the current APIs might not turn out well

That's how I recall it. The only cases we really considered were Python
modules (source code plus pyc) and extension modules (no source, loader
should return None).

Paul

> So, where is the filename coming from?

Python 3.7.0 (default, Jul 23 2018, 10:07:21)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from sage.all import Integer
>>> try:
...     Integer(1)/Integer(0)
... except Exception as e:
...     E = e
>>> E.__traceback__.tb_next.tb_frame.f_code.co_filename
'sage/structure/element.pyx'

So this is the correct filename, relative to site-packages.

> Now, though, we need a way for SageMath to get it working on releases up to and including 3.7, *without* any help from Cython or CPython

That's not really what I need. We already have a work-around in SageMath:

try:
    from importlib.machinery import ExtensionFileLoader
except ImportError:
    pass  # Python 2
else:
    del ExtensionFileLoader.get_source

So this issue is about finding a proper solution, which doesn't require a monkey-patch like the above. And I want it to work without any special support from the package, installing .pyx sources should "just work". This issue is not specific to SageMath, it's just that SageMath is the only project that I know which actually installs .pyx sources.

> Define a subclass of ExtensionModuleLoader [1] that overrides get_source() to also look for a ".pyx" file adjacent to the extension module.

As I mentioned before, get_source() cannot work because a single Cython module can have multiple sources. So whatever method is used to retrieve the Cython sources must know the filename; the module is not sufficient.

As you can see from the monkey-patch, not implementing get_source() at all works, but that's probably more or less by accident. I'm not sure to what extent that could be documented as the official way to look up a filename relative to sys.path entries.

> One other thing I will say is that the PEP 302 APIs were seemingly designed for working with Python source and were not necessarily oriented towards alternative code representations that were executed

That's also my impression. I'm perfectly fine with saying that the get_source() method defined in PEP 302 only applies to Python sources.

That, together with the fact that we are really looking for *filenames* not *modules* is why the fix should involve the linecache module and not PEP 302. In https://bugs.python.org/issue32797#msg323106 Nick seemed to agree with that.

If you're OK with that, I don't see the problem. My objection was with the claims that ExtensionLoader.get_source shouldn't return None or shouldn't exist. PEP 302 mandates that loaders have a get_source, and that get_source should return None if the loader can't supply source. And that's precisely what ExtensionLoader is doing.

So if we're agreed that there's no changes needed to PEP 302 (more accurately, the loader protocol as it exists now) or to ExtensionLoader, then I'm happy to let others work out a solution on that basis.

> If you're OK with that, I don't see the problem.

I recall that we already agreed several months ago, when I submitted https://github.com/python/cpython/pull/6653

I just realized the existence of https://docs.python.org/3/library/importlib.html#importlib.abc.ResourceReader

Isn't this *exactly* what we need here?

> SageMath is the only project that I know which actually installs .pyx sources.

Ah, right. I wrongly remembered that they are automatically included in binary packages, but that only applies to .py source of Cython compiled Python modules (which are obviously included), not for .pyx sources. Those still need to be explicitly included, which suggests that most projects wouldn't do that (especially because it doesn't help, given this very issue).


> The one possible fly in the ointment is if there are use cases that we
> need to support where a single .so/.pyd file is built from *multiple*
> source files, as get_source doesn't allow for that.

That probably applies to many, if not most, extension modules. For Cython it's .pyx, .pxi and .pxd files, plus any external C/C++ source files, for C it's .c/.cpp and .h files, other code generators have their own (set of) source files, many wrapper extension modules out there consist of more than one source file that would be relevant, …

I would consider .py files the exceptional case here.

I think I could make linecache use the ResourceReader API to find the source file instead of looking in the sys.path entries. However, that's orthogonal to PR 6653: we still need PR 6653 to continue looking for the source file in the first place.

Brett:
> As Nick said, we have no generalized concept of source maps and I think
coming up with that is what will be required to solve this as i personally
don't view Loader.get_source() is not meant to be a generalized concept of
some form of source code but Python source code.

I see what you're saying here, but given that Loader can return Python modules that are ostensibly not actually generated from Python source code, then it's not *obvious* that Loader.get_source() must return Python source.  At the very least the documentation [1] should clarify this.  But it's also a bit arbitrarily limiting, especially given the demonstrable possibility of providing tracebacks and code inspection for *non-Python* code (as in the case of Cython) that compiles to Python modules.


Nick:
> 1. Enhance PathFinder to allow specification of which path importer cache and path_hooks entries to use

This would be good.  Perhaps veering off-topic, but in an earlier attempt to fix this issue I actually tried to implement a sys.path_hooks hook for importing Cython modules such that I could provide an appropriate Loader for them with a get_source that actually works.  This turned out to be very difficult in large part due to the difficulty of customizing the relationship between the default PathFinder on sys.meta_path and the sys.path_hooks entries.  I made a post about this to python-ideas [2] but it never gained much traction, probably in large part due to the specialized nature of the problem and the complexity of my solution :)


[1] https://docs.python.org/3/library/importlib.html#importlib.abc.InspectLoader.get_source
[2] https://mail.python.org/pipermail/python-ideas/2018-February/048905.html

To add, while an enhancement just to linecache would make sense in its own right, I don't see the problem with also extending the Loader.get_source() API to be more useful as well.  Its current behavior is to just return a string (or None), but it seems to me one could keep that basic behavior, but also extend it to optionally return a more sophisticated source map data structure (that could involve multiple source files as well).

This could even be useful for built-in modules.  I would love, for example, to be able to get stack traces within extension modules integrated into Python tracebacks if they are compiled with some appropriate debug flags.  The case of Cython demonstrates that something like this is perfectly doable.

> To do that, I believe it can be made to work in much the same way it did in Python 2 if SageMath were to do the following:
>
> 1. Define a subclass of ExtensionModuleLoader [1] that overrides get_source() to also look for a ".pyx" file adjacent to the extension module. This would also look for any of the file suffixes in SOURCE_SUFFIXES if a .pyx file isn't found.
> 2. Create an instance of FileFinder [3] that uses the custom loader subclass for any of the file suffixes in EXTENSION_SUFFIXES [4]
> 3. Replace the regular file finding hook in sys.path_hooks with  the path_hook method from that new FileFinder instance (by default, there's only one FileFinder hook installed, and it can currently be identified as "obj.__name__ == 'path_hook_for_FileFinder')
> 4. Invalidate importlib's caches, so any future extension module imports will use the custom extension module loader, rather than the default one

This is pretty edifying, because Nick's idea is almost exactly what I did six months ago :)  https://git.sagemath.org/sage.git/diff/?id2=0a674fd488dcd7cb779101d263c10a874a13cf77&id=8b63abe731c510f0de9ef0e3ab9a0bda3669dce1

Turned out to be very non-trivial of course, and I believe it should not have been as complicated as it was.

It also still doesn't solve the problem that Loader.get_source does not support multiple source files, which Cython code may have (a .pyx and a .pxd being a common case).  I'm glad Paul Moore seems to also agree (now that I've actually read the rest of the thread) that the ExtensionLoader.get_source, at the very least, could be made more useful.  Whatever form that takes would be worth extending to other loaders that implement get_source as well...

Or, define a new "get_sourcemap()" method that could return additional metadata, e.g. a line number mapping.

> I would love, for example, to be able to get stack traces within extension modules integrated into Python tracebacks if they are compiled with some appropriate debug flags.

Awesome idea, I want to work on that :-)

This should be possible using the backtrace() and backtrace_symbols() functions, which exist at least in GNU libc and on OSX.

> Or, define a new "get_sourcemap()" method that could return additional metadata, e.g. a line number mapping.

What's your use case for that? I am asking because it might make more sense for get_source() (or whatever its replacement is) to return the already-mapped sources.

I pushed a documentation-only patch on PR 9540 to better document status quo. Can somebody please review either PR 6653 or PR 9540?

New changeset 057f4078b044325dae4af55c4c64b684edaca315 by Petr Viktorin (jdemeyer) in branch 'master':
bpo-32797: improve documentation of linecache.getline (GH-9540)
https://github.com/python/cpython/commit/057f4078b044325dae4af55c4c64b684edaca315