libgap: do not restrict libgap.<tab> completion to a fixed list

[nthiery]

Since #19917, tab completion on libgap returns a fixed list. As noted there, new global functions and objects introduced when loading packages remain therefore hidden. Now that loading packages in libgap is much better supported, this is really becoming an issue.

Proposal, after discussion with GAP devs: use the same implementation as in the plain gap interface (namely ask GAP for the list of global names), without a cache.

Pros:

• Principle of less surprise: keep plain GAP, Sage's gap and Sage's libgap consistent;

• Follow the design decisions of GAP to take advantage of their expertise;

• Supports loading packages and advertising their content (this support was postponed to a later ticket in Libgap tab completion #19917);

• No need any more to maintain in Sage a list of "important" GAP functions.

• With libgap, fetching the list of global GAP symbols is quasi instantaneous:

  sage: %time a = [str(x) for x in libgap.NamesSystemGVars()]
  CPU times: user 21.2 ms, sys: 0 ns, total: 21.2 ms
  Wall time: 20.9 ms
  

  Hence the implementation is simple; we need not worry about caching, invalidating the cache upon loading packages, ...

Con:

• This makes for a larger tab completion list

CC: @vbraun @tscrim @dimpase @embray

Component: interfaces

Keywords: gap libgap

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

[nthiery]

comment:1

If we agree that this is the way to go, I can take care of the implementation.

[dimpase]

comment:2

What I'd like to understand whether with this approach there is a performance hit if one
uses libgap.Blah calls in a loop.
Calling (lib)GAP inside a loop to determine the function to call would quickly become very slow.

[nthiery]

comment:4

Hi Dima,

What I'd like to understand whether with this approach there is a performance hit if one
uses libgap.Blah calls in a loop.
Calling (lib)GAP inside a loop to determine the function to call would quickly become very slow.

This ticket is only about tab completion (VIA dir(self)), and is orthogonal to attribute access (via __getattr__). So this should have no influence whatsoever on libgap.Blah calls.

[embray]

comment:5

What is the relationship between this ticket, and #27911 which is older?

[dimpase]

comment:6

their exact relationship status is "legally separated" :-)

[nthiery]

comment:7

:-)

#27911 is about attribute access libgap.xxx whenever xxx exists in GAP. This one is about what should appear in the tab-completion on libgap. Of course some level of consistency is good, but other than this they are independent issues.

[embray]

comment:8

I see, yes that's a subtle difference. In practice there's a lot of overlap in terms of the implementation details.

I've never quite understood the current implementation that uses (and limits one to) a hard-coded list. I remember asking about this on sage-devel a while ago when I was first dealing with it, and nobody had a very concrete answer, including Volker who I think wrote it (not sure though).

Would definitely give +1 to a better solution if there is one.

[vbraun]

comment:9

Just repeating my comment from #27911, this tends to cough up a lot of private / undocumented functions in GAP that necessarily are in the public GAP namespace but users aren't supposed to access.

The current implementation uses IsDocumentedWord to only return documented symbols. Which also is slow as molasses since it grinds through the documentation, so it needs to be cached.

On the plus side, gap itself tab-completes over everything so at least we'd match that bug/misfeature/feature (pick your choice). So I'm fine with changing it...

[nthiery]

comment:10

Thanks Volker for the feedback!

Agreed, displaying private functions is on the misfeature side. Hopefully that will eventually get fixed on the GAP side.

If no one objects I'll make libgap's completion consistent with GAP's later this week or during Cernay.

[embray]

comment:11

I agree with Volker that it's a shame that GAP's global namespace is so overloaded, but that's a (mis)feature of GAP that it doesn't ultimately help us to fight against.

Perhaps as a policy, when using GAP code internally in Sage only those functions for which IsDocumentedWord is true should be used. Not sure if there's a great way to enforce that except by checking manually.

[nthiery]

comment:12

Replying to @embray:

I agree with Volker that it's a shame that GAP's global namespace is so overloaded, but that's a (mis)feature of GAP that it doesn't ultimately help us to fight against.

Ok! I'll proceed then. Thanks for feedback.

Perhaps as a policy, when using GAP code internally in Sage only those functions for which IsDocumentedWord is true should be used. Not sure if there's a great way to enforce that except by checking manually.

+1

[embray]

comment:13

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

[embray]

Changed keywords from none to gap libgap

[dimpase]

comment:15

Anything on this in view of gappy ?

[embray]

comment:17

It's been on my todo list but I didn't make an explicit issue for it; thanks for reminding me: embray/gappy#15