make sage_getsourcelines and sage_getfile consistent

[nbruin]

We presently have:

sage: sage_getsource(hyperbolic_triangle) in open(sage_getfile(hyperbolic_triangle),'r').read()
False

The problem is that hyperbolic_triangle is wrapped and its source is provided by a _sage_src_lines_() method, but on the getfile front, there is no analogue, so the file isn't found. That means edit doesn't work properly.

One solution is to provide a parallel method _sage_src_file_(). Another, which requires more editing, is to change sage_getsourcelines to return a triple ([lines],lineno,filename) instead of ([lines],lineno) as it does now (it's a little strange to return a line number without the file into which this indexes anyway). Note that filename could end up being something that doesn't actually open, because interactively defined classes get funny "filenames" that, via the inspect linecache, actually can used to index source lines without having a file correspond to them.

CC: @simon-king-jena @nthiery @roed314

Component: misc

Branch/Commit: u/nbruin/ticket/16382 @ 31c39fa

Issue created by migration from https://trac.sagemath.org/ticket/16382

[nbruin]

Branch: u/nbruin/ticket/16382

[nbruin]

comment:1

OK, first try for implementing a _sage_src_file_ helper protocol. Note that I have made its presence a requirement for now if _sage_src_lines_ is present, because the other source file heuristics don't have much of a chance if they don't work.

[nbruin]

comment:2

Also, note:

sage: P.<x,y> = QQ[]
sage: P._sage_src_lines_()
TypeError: _sage_src_lines() takes no arguments (1 given)

which is currently relied on to detect a dynamic class instance. The method in question tries to make a static method, but the subsequent inclusion into a dynamic class dictionary seems to break this (hence, the method binds anyway and the call receives an unexpected self argument).

If one fixes _sage_src_lines (by for instance accepting an optional self argument), source retrieval gets broken! So I think the current implementation of _sage_src_lines is actually wrong. It should either be removed (and the rest of the infrastructure adapted to detect dynamic classes in a different way--which is possible, because its current desired behaviour is apparently to raise a TypeError on instances of dynamic classes, which could be refactored), or it should be implemented to return the correct contents. Of course _sage_src_file could then be corrected similarly.

Attempt to show that _sage_src_lines_ doesn't return something useful:

sage: P._sage_src_lines_
<bound method JoinCategory.parent_class._sage_src_lines of Multivariate Polynomial Ring in x, y over Rational Field>
sage: P.__dict__['_sage_src_lines_']
<staticmethod at 0x3dec9b8>
sage: P.__dict__['_sage_src_lines_'].__func__()
(['    class ParentMethods:\n',
  '        """\n',
  '        Put methods for parents here.\n',
  '        """\n',
  '        pass\n'],
 1096)

As you can see:

• a static method in an instance dictionary binds anyway
• the source returned is not of the object we're interested in

---

New commits:

4427521

trac 16382: experimental implementation for improved source file introspecion

[nbruin]

Commit: 4427521

[nbruin]

comment:3

The implementation of _sage_src_lines was introduced in #5991, so I'm CC-ing nthierry and roed as author/reviewer of that change. They may be able to comment on how the routine should be implemented and where that routine should be finding its source.

[sagetrac-git]

Changed commit from 4427521 to 31c39fa

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

31c39fa

trac 16382: fix the dynamic_class._sage_src_lines etc. to return appropriate values on a cdef derived class, rather than raising a TypeError

[nbruin]

comment:5

This commit should fix the problem for dynamic classes derived from cdef classes. For those, the _sage_src_lines_ methods end up in the __dict__ and they get bound! (probably due to a custom getattr that doesn't distinguish between methods and staticmethod). In fact, since the doccls value that is available at the time the closure of the relevant method is constructed doesn't have a useful value anyway, this behaviour is even somewhat useful: we can then look up the source on self.__class__ (which then does have a useful value).

[nbruin]

comment:6

In case someone wants to fix the fact that staticmethods on dynamicclasses end up binding anyway: I suspect the problem occurs somewhere in getattr_from_other_class, and it's probably an issue that the "getter" gets used twice. To illustrate:

sage: class T(object):
    @staticmethod
    def stat():
        pass
....:     
sage: t=T()
sage: t.stat
<function __main__.stat>
sage: T.__dict__['stat']
<staticmethod at 0x7009210>
sage: T.__dict__['stat'].__get__(t)
<function __main__.stat>
sage: T.__dict__['stat'].__get__(t).__get__(t)
<bound method ?.stat of <__main__.T object at 0x6ff0510>>

As you can see, the standard staticmethod getter returns the wrapped function object. However, functions ALWAYS have a getter method that binds them. So if an staticmethod attribute inadvertently is "got" twice (first retrieved from the "other" class, and then passed through its own getter another time), one would end up binding a static method. Fixing this is not within the scope of this ticket, though.

[fchapoton]

comment:8

failing doctest, see patchbot report