Opened 15 months ago
Last modified 2 months ago
#31632 new defect
tab-completion of cached_method in extension class crashes SageMath
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-9.7 |
Component: | misc | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Tab completion on the prefix of a cached_method
of an extension class not in Sage source code provokes of crash. See https://gitlab.com/videlec/sage_tab_completion_bug to reproduce.
The bug was confirmed reproducibly on
- archlinux sage 9.3.rc2
- online cocalc with sage 9.2
The bug is not present on
- docker sage 8.8 and 9.1.rc2
Workaround
Compile your Cython code with
import Cython.Compiler.Options Cython.Compiler.Options.embed_pos_in_docstring = True
Tasks
- #31643: about tab-completion accessing attributes
- fix sage inspection module
Change History (33)
comment:1 Changed 15 months ago by
- Description modified (diff)
- Priority changed from blocker to critical
comment:2 follow-up: ↓ 4 Changed 15 months ago by
- Description modified (diff)
comment:3 Changed 15 months ago by
- Description modified (diff)
comment:4 in reply to: ↑ 2 Changed 15 months ago by
Replying to dimpase:
This is already in 9.2 (reproduced following the instructions in the gitlab fork)
Thanks for testing. I believe that it is a Python3 thing (in particular quite old).
comment:5 Changed 15 months ago by
- Description modified (diff)
comment:6 Changed 15 months ago by
- Description modified (diff)
comment:7 Changed 15 months ago by
- Description modified (diff)
comment:8 Changed 15 months ago by
On conda (with sage 9.2) I get the error but no crash
Traceback (most recent call last): File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/cache.py", line 110, in wrapper return dct[key] KeyError: ((), frozenset()) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/cache.py", line 110, in wrapper return dct[key] KeyError: (('py__doc__',), frozenset()) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/sage/misc/sageinspect.py", line 2378, in sage_getsourcelines return inspect.getsourcelines(obj) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 955, in getsourcelines lines, lnum = findsource(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 768, in findsource file = getsourcefile(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 684, in getsourcefile filename = getfile(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 666, in getfile type(object).__name__)) TypeError: module, class, method, function, traceback, frame, or code object was expected, got method_descriptor During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/sage/misc/sageinspect.py", line 2378, in sage_getsourcelines return inspect.getsourcelines(obj) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 955, in getsourcelines lines, lnum = findsource(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 768, in findsource file = getsourcefile(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 684, in getsourcefile filename = getfile(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 653, in getfile raise TypeError('{!r} is a built-in class'.format(object)) TypeError: <class 'method_descriptor'> is a built-in class During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/IPython/terminal/ptutils.py", line 115, in get_completions yield from self._get_completions(body, offset, cursor_position, self.ipy_completer) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/IPython/terminal/ptutils.py", line 131, in _get_completions for c in completions: File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/IPython/core/completer.py", line 438, in _deduplicate_completions completions = list(completions) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/IPython/core/completer.py", line 1827, in completions for c in self._completions(text, offset, _timeout=self.jedi_compute_type_timeout/1000): File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/IPython/core/completer.py", line 1884, in _completions signature = _make_signature(jm) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/IPython/core/completer.py", line 993, in _make_signature signatures = completion.get_signatures() File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/api/classes.py", line 576, in get_signatures for s in self._get_signatures() File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/api/classes.py", line 565, in _get_signatures return [sig for name in names for sig in name.infer().get_signatures()] File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/api/classes.py", line 565, in <listcomp> return [sig for name in names for sig in name.infer().get_signatures()] File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/base_value.py", line 512, in get_signatures return [sig for c in self._set for sig in c.get_signatures()] File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/base_value.py", line 512, in <listcomp> return [sig for c in self._set for sig in c.get_signatures()] File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/value.py", line 138, in get_signatures _, return_string = self._parse_function_doc() File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/cache.py", line 112, in wrapper result = method(self, *args, **kwargs) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/value.py", line 146, in _parse_function_doc doc = self.py__doc__() File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/value.py", line 116, in py__doc__ return self.access_handle.py__doc__() File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/subprocess/__init__.py", line 386, in _workaround return self._cached_results(name, *args, **kwargs) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/cache.py", line 112, in wrapper result = method(self, *args, **kwargs) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/subprocess/__init__.py", line 390, in _cached_results return self._subprocess.get_compiled_method_return(self.id, name, *args, **kwargs) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/subprocess/functions.py", line 27, in get_compiled_method_return return getattr(handle.access, attribute)(*args, **kwargs) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/jedi/inference/compiled/access.py", line 189, in py__doc__ return inspect.getdoc(self._obj) or '' File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 601, in getdoc doc = object.__doc__ File "sage/docs/instancedoc.pyx", line 211, in sage.docs.instancedoc.InstanceDocDescriptor.__get__ (build/cythonized/sage/docs/instancedoc.c:1800) return self.instancedoc(obj) File "sage/misc/cachefunc.pyx", line 877, in sage.misc.cachefunc.CachedFunction._instancedoc_ (build/cythonized/sage/misc/cachefunc.c:5026) sourcelines = sage_getsourcelines(f) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/sage/misc/sageinspect.py", line 2397, in sage_getsourcelines return sage_getsourcelines(obj.__class__) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/sage/misc/sageinspect.py", line 2395, in sage_getsourcelines return sage_getsourcelines(B) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/sage/misc/sageinspect.py", line 2398, in sage_getsourcelines raise err File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/site-packages/sage/misc/sageinspect.py", line 2378, in sage_getsourcelines return inspect.getsourcelines(obj) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 955, in getsourcelines lines, lnum = findsource(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 768, in findsource file = getsourcefile(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 684, in getsourcefile filename = getfile(object) File "/home/vincent/miniconda3/envs/flatsurvey/lib/python3.7/inspect.py", line 653, in getfile raise TypeError('{!r} is a built-in class'.format(object)) TypeError: <class 'object'> is a built-in class
comment:9 Changed 15 months ago by
- Description modified (diff)
comment:10 Changed 15 months ago by
- Description modified (diff)
comment:11 Changed 15 months ago by
I have also had problems with cached_method
in Cython code, in the past. As far as I recall, it is necessary to define a public __cached_methods
attribute for them to work. I have quickly checked it on your example though and it did not seem to resolve the issue, so this may be a different problem.
comment:12 Changed 15 months ago by
You should in addition inherit from SageObject
, when using cached_method
(or at least try), but this also did not resolve the issue.
comment:13 Changed 15 months ago by
Indeed this seems to have appeared at some point recent.
On debian buster I can't reproduce it with sage 9.1, at least not the hard crash. The underlying problem to fetch a.two.__doc__
is way older, but hasn't lead to a hard crash.
This seems to be related #31480 (which was a blocker, because as you noted hard crashes are annoying and it had a super easy fix). This tab completion hard crash also appeared with some ipython upgrade or so that happened recently.
comment:14 follow-up: ↓ 15 Changed 15 months ago by
I think the mentioned workaround is also a possible solution. You cannot just cythonize a file and then except it to work completely nice with sage. It should be clear that you need to run a special cythonize function that enables the proper defaults for sage (e.g. cython aliases etc).
However, I can't find such a function and I think sage should provide a customized cythonize_for_sage
that sets the proper arguments (also we usually assume cdivison=True
etc, I don't know if this is respected in your case either).
Or at least provide the cythonize arguments somewhere...
Am I making any sense? Anyway, I think this is out of scope for sage 9.3
comment:15 in reply to: ↑ 14 Changed 15 months ago by
Replying to gh-kliem:
I think the mentioned workaround is also a possible solution. You cannot just cythonize a file and then except it to work completely nice with sage. It should be clear that you need to run a special cythonize function that enables the proper defaults for sage (e.g. cython aliases etc).
However, I can't find such a function and I think sage should provide a customized
cythonize_for_sage
that sets the proper arguments (also we usually assumecdivison=True
etc, I don't know if this is respected in your case either).Or at least provide the cythonize arguments somewhere...
Am I making any sense? Anyway, I think this is out of scope for sage 9.3
I see at least three problems:
- Access to the documentation should not trigger the access the source code. The
CachedFunction._instancedoc_
makes a call tosage_getsourcelines
if the line in the source code is not available. This is exactly what the workaround in the ticket description provides.
- Accessing the documentation/source should never result in a crash
- Why on earth tab-completion have anything to do with access to the documentation?
I believe there is work to be done on different fronts: the sageinspect
module, the CachedFunction._instancedoc_
and understand how the tab-completion works.
comment:16 follow-up: ↓ 18 Changed 15 months ago by
Ok, yes, I agree with you.
- Sage crashing is very unfortunate behavior and it definitely shouldn't happen because some trivial python error occurs.
- #31480 only fixed the symptoms. The point above applies there as well. But also tab completion should not even trigger loading lazy imports (unless your actually tab completing within the lazy imported thing and then of course it needs to grep the signature via the
inspect
module and maybe this needs to access source or something like this).
To verify that tab completion is doing too much one can do the following:
Start sage fresh
sage: type(polytopes._object) <class 'NoneType'>
Hit tab after pol
. polytopes
isn't even your first guess, but it's on the list. Now repeat:
sage: type(polytopes._object) <class 'sage.geometry.polyhedron.library.Polytopes'>
So tab completion does not work well with lazy imports at all.
comment:17 Changed 15 months ago by
Certainly unrelated: On my machine, when I do a copy/paste of a sage prompt line I also get a bunch of white spaces in the buffer. The same happened in your examples from 16 (I removed the spaces manually). This behavior was not there before.
comment:18 in reply to: ↑ 16 ; follow-up: ↓ 19 Changed 15 months ago by
Replying to gh-kliem:
To verify that tab completion is doing too much one can do the following:
Start sage fresh
sage: type(polytopes._object) <class 'NoneType'>Hit tab after
pol
.polytopes
isn't even your first guess, but it's on the list. Now repeat:sage: type(polytopes._object) <class 'sage.geometry.polyhedron.library.Polytopes'>So tab completion does not work well with lazy imports at all.
For me it is even stranger: before the tab and and after the first I still get None
. Only on the second run it gets lazily imported.
comment:19 in reply to: ↑ 18 Changed 15 months ago by
Replying to vdelecroix:
Replying to gh-kliem:
To verify that tab completion is doing too much one can do the following:
Start sage fresh
sage: type(polytopes._object) <class 'NoneType'>Hit tab after
pol
.polytopes
isn't even your first guess, but it's on the list. Now repeat:sage: type(polytopes._object) <class 'sage.geometry.polyhedron.library.Polytopes'>So tab completion does not work well with lazy imports at all.
For me it is even stranger: before the tab and and after the first I still get
None
. Only on the second run it gets lazily imported.
Yes, you are right. I got this wrong. If you tab complete p
twice, then everythings neatly lazy imported that starts with a p
is being imported.
comment:20 Changed 15 months ago by
Note that tab completion runs in a separate thread, which previously caused trouble in #22704
comment:21 Changed 15 months ago by
Not reproducible with
from IPython import get_ipython ip = get_ipython() c = ip.Completer ip.ex("from sage.all import *") for _ in range(4): print(c.all_completions("pol")) print(ip.ex('type(polytopes._object)'))
which prints four times
['polar_plot', 'polygen', 'polygens', 'polygon', 'polygon2d', 'polygon3d', 'polygon_spline', 'polygonal_number', 'polygons3d', 'polylog', 'polymake', 'polytopes'] None
comment:22 Changed 15 months ago by
Replying to mkoeppe:
Note that tab completion runs in a separate thread, which previously caused trouble in #22704
Ah, this probably explains, why it takes two times, until you see the effect.
You do this effect immediatly in memory usage. Tab completion of h
takes something around 70 MB of memory. How stupid is that.
Also tab completion of l
gives:
sage: l// Giac share root-directory:/home/jonathan/Applications/sage/local/share/giac/ // Giac share root-directory:/home/jonathan/Applications/sage/local/share/giac/ Help file /home/jonathan/Applications/sage/local/share/giac/doc/fr/aide_cas not found Added 0 synonyms
And tab completion twice s
has sage crash hard again. The essense of it being
AttributeError: module 'sage.coding.linear_code' has no attribute 'self_orthogonal_binary_codes'
because it is incorrectly lazily imported in src/sage/coding/all.py
. Implying three things for me:
- Again, sage should not crash hard because of such a silly thing.
- We should have a doctest somewhere that unrolls all lazy imports, just to make sure that they actually work. Apparently this is not being tested.
- Of course it should be fixed in
src/sage/coding/all.py
.
comment:23 Changed 15 months ago by
In the first place, tab completion should not trigger the lazy imports... Why is it doing so?
comment:24 Changed 15 months ago by
I mentioned this above and agree. I don't know. For the moment (and all I'll do today), I just remarked that this is clearly wrong.
If you autocomplete within a lazy imported thing as in polytopes.cube(int
(because you are too lazy to type the argument intervals
), you need to fetch the signature, which will probably trigger an import. However, just an import of that one class is justified. Otherwise it should not trigger any import (it only needs the names after all).
comment:25 Changed 15 months ago by
Concerning lazy imports, I think I got it. Since some recent Python3 versions, objects could be wrapped and in such case the object implements __wrapped__
(this is for example the case with some decorators in functools). Ipython tries to be smart with this with respect to tab completion and hence check for __wrapped__
. Doing this on a lazy_import
will trigger the import.
comment:26 Changed 15 months ago by
Nope. That could not explain...
comment:27 Changed 15 months ago by
With
-
src/sage/misc/lazy_import.pyx
diff --git a/src/sage/misc/lazy_import.pyx b/src/sage/misc/lazy_import.pyx index 336567e22c..49abc0b849 100644
a b cdef class LazyImport(object): 325 325 sage: my_integer.sqrt is Integer.sqrt 326 326 True 327 327 """ 328 print('getattr(attr={}) of self={}'.format(attr, self)) 328 329 return getattr(self.get_object(), attr) 329 330 330 331 # We need to wrap all the slot methods, as they are not forwarded
you can have a look at what is asked to your lazy_import
objects.
Here is what I got (up to desynchronization)
getattr(attr=Algebras) of self=<class 'sage.categories.groups.Groups'> getattr(attr=Finite) of self=<class 'sage.categories.groups.Groups'> getattr(attr=Finite) of self=<class 'sage.categories.groups.Groups'> sage: pol<TAB> getattr(attr=__module__) of self=<built-in function polygon_spline> getattr(attr=__module__) of self=<built-in function polygon_spline> getattr(attr=__module__) of self=<built-in function polygon_spline> getattr(attr=__dict__) of self=<built-in function polygon_spline> getattr(attr=__wrapped__) of self=<built-in function polygon_spline> getattr(attr=__signature__) of self=<built-in function polygon_spline> getattr(attr=_partialmethod) of self=<built-in function polygon_spline> getattr(attr=__name__) of self=<built-in function polygon_spline> getattr(attr=__code__) of self=<built-in function polygon_spline> getattr(attr=__defaults__) of self=<built-in function polygon_spline> getattr(attr=__kwdefaults__) of self=<built-in function polygon_spline> getattr(attr=__annotations__) of self=<built-in function polygon_spline> getattr(attr=__text_signature__) of self=<built-in function polygon_spline> getattr(attr=__module__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__module__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__module__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__dict__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__wrapped__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__signature__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=_partialmethod) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__name__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__code__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__defaults__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__kwdefaults__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__annotations__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20> getattr(attr=__text_signature__) of self=<sage.geometry.polyhedron.library.Polytopes object at 0x7f21a0b12e20>
The three getattr(attr=XXX) of self=<class 'sage.categories.groups.Groups'>
are definitely unrelated but worth looking at since they should not be there! I thought that resolution of lazy import was forbidden on startup.
comment:28 Changed 15 months ago by
All right, it has to do with extension class. My 25 was looking in the good direction: IPython does call stuff on the object in some cases.
sage: cython(""" ....: cdef class A(object): ....: def __getattr__(self, key): ....: print('getattr(key={})'.format(key)) ....: raise AttributeError ....: """) sage: a = A() sage: my_custom_name = A() sage: from IPython import get_ipython sage: ip = get_ipython() sage: ip.Completer.all_completions("my_custo") getattr(key=__module__) getattr(key=__module__) ['my_custom_name']
comment:30 Changed 14 months ago by
- Milestone changed from sage-9.3 to sage-9.4
comment:31 Changed 10 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:32 Changed 6 months ago by
- Milestone changed from sage-9.5 to sage-9.6
comment:33 Changed 2 months ago by
- Milestone changed from sage-9.6 to sage-9.7
This is already in 9.2 (reproduced following the instructions in the gitlab fork)