Opened 3 years ago

Closed 2 years ago

#24998 closed enhancement (fixed)

py3: incorporate a tab completion function from sagenb

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.2
Component: python3 Keywords:
Cc: embray, jdemeyer Merged in:
Authors: Frédéric Chapoton Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: dc2e6f0 (Commits) Commit: dc2e6f08d55e54eb9639fa98965f0901c5d31882
Dependencies: Stopgaps:

Description

to continue cutting the links to sagenb

Change History (24)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/24998
  • Commit set to 9bf9f05f1f56d2e75439c2d2169459c3dfc2c0a3
  • Status changed from new to needs_review

comment:2 Changed 3 years ago by git

  • Commit changed from 9bf9f05f1f56d2e75439c2d2169459c3dfc2c0a3 to cdfe0565defebdfe26eb222472698bb58ca9657e

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

cdfe056incorporate one function from sagenb

comment:3 Changed 3 years ago by chapoton

  • Cc embray jdemeyer added

and bot is morally green

comment:4 Changed 3 years ago by chapoton

ping ?

comment:5 Changed 3 years ago by jhpalmieri

How about some doctests for completions?

comment:6 Changed 3 years ago by git

  • Commit changed from cdfe0565defebdfe26eb222472698bb58ca9657e to 039799b174caf91223769ea52d397fd09352cb0c

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

039799badding a doctest

comment:7 Changed 3 years ago by chapoton

done

comment:8 Changed 3 years ago by chapoton

and bot is morally green again

comment:9 Changed 3 years ago by embray

Why not go ahead and make this Python 3 compatible as well (currently I think it isn't)? Otherwise +1 from me.

comment:10 Changed 3 years ago by chapoton

Could you precise in which way it is not py3 compatible ? I do not feel very qualified to make it so, by the way. I am quite lost in what exactly is using unicode and what else is using bytes..

comment:11 Changed 3 years ago by embray

Just try testing it on Python 3, I mean. For example, this will break:

            v += [x for x in __builtins__.keys() if x[:n] == s] 

Actually the line is broken on Python 2 as well--there is no __builtins__.keys(). Maybe it means vars(__builtins__)? I'm not sure though. I haven't read the code that closely.

comment:12 Changed 3 years ago by git

  • Commit changed from 039799b174caf91223769ea52d397fd09352cb0c to da202c8daa2d91d4dca718e5234dd96ec202545f

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

da202c8simplify the completion method

comment:13 Changed 3 years ago by chapoton

Here is a tentative of simplification. Now pyflakes is happy.

comment:14 Changed 3 years ago by chapoton

and bot is morally green again

comment:15 Changed 3 years ago by jhpalmieri

I'm happy with it.

comment:16 Changed 3 years ago by chapoton

Should I remove the unused option ?

comment:17 Changed 3 years ago by jhpalmieri

Would that require deprecation?

comment:18 Changed 3 years ago by chapoton

No, because we are not removing the original method in sagenb, just making inside sage a minimal version suited for our purposes.

comment:19 Changed 3 years ago by chapoton

So would you give a positive review in the present state, or should I remove the ignored argument ?

comment:20 Changed 3 years ago by jhpalmieri

It seems best to remove the ignored argument, but I don't feel strongly about it.

comment:21 Changed 3 years ago by git

  • Commit changed from da202c8daa2d91d4dca718e5234dd96ec202545f to dc2e6f08d55e54eb9639fa98965f0901c5d31882

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

9afe56fMerge branch 'u/chapoton/24998' in 8.2.rc1
dc2e6f0trac 24998 removing ignored argument

comment:22 Changed 3 years ago by chapoton

The ignored argument has now been removed.

comment:23 Changed 3 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Okay, looks good.

comment:24 Changed 2 years ago by vbraun

  • Branch changed from u/chapoton/24998 to dc2e6f08d55e54eb9639fa98965f0901c5d31882
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.