Opened 10 years ago

Last modified 10 years ago

#13678 closed enhancement

Allow tab completion of matrix constructor — at Version 5

Reported by: Robert Bradshaw Owned by: jason, was
Priority: major Milestone: sage-5.7
Component: linear algebra Keywords:
Cc: Charles Bouillaguet Merged in:
Authors: Robert Bradshaw, Volker Braun Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Volker Braun)

E.g. matrix.identity(...), matrix.random(...), matrix.load(...) etc. all discoverable by tab completion.

Apply 13678-matrix-methods_vb.patch

Change History (7)

Changed 10 years ago by Robert Bradshaw

Attachment: 13678-matrix-methods.patch added

comment:1 Changed 10 years ago by Robert Bradshaw

Status: newneeds_review

comment:2 Changed 10 years ago by Rob Beezer

Very nice! I discovered a couple constructors I didn't know about (just reading the patch). I'll add it to my queue of things to review, but anybody else should feel free to beat me to it.

comment:3 Changed 10 years ago by Rob Beezer

I like it! Works as advertised. I'll run tests shortly. In the meantime, a suggestion and a question.

(1) Stripping out "matrix" from the name of the method is a nice touch, but then the docstring appears (to the novice) to be talking about a slightly diffferent command than the one queried. Having ready access to constructors is the first step in experimenting with Sage, then tab-completion takes over for methods on that object. So tab-completion to discover constructors is fabulous. But it should be as totally straightforward as possible, IMHO.

What do you think of putting "matrix" back in the method names of the matrix object? Yes, it is verbose and redundant. It does not seem to complicate tab-completion (ie, you do not need to use tab any more in either scenario). And in the Sage library, authors can use the "old" method names without involving the matrix object (and I would think this would be preferable).

(2) Is there anyway to make the func_* methods on this object invisible on tab-completion? I have no real good idea if they are useful, or just detritus. It'd be great if matrix.<tab> only showed the constructors.

Rob

comment:4 Changed 10 years ago by Robert Bradshaw

Both valid points of feedback.

(1) I'd lean towards not putting matrix back in the name; what if we modified some of the examples to use the new format as well? Would that make things clear that we're talking about the "same function." We could also (automatically) add a line in the docstring that "This function is available as matrix.identity or identity_matrix"

(2) Short of hiding them for all functions (-1 to that) another option is to matrix() a __call__ method on a class. If you want I can post a new patch doing this.

Changed 10 years ago by Volker Braun

Improved patch

comment:5 Changed 10 years ago by Volker Braun

Authors: Robert Bradshaw, Volker Braun
Description: modified (diff)
Reviewers: Volker Braun

Great idea to use a decorator! I've implemented the __call__ version. Also, I gave the decorator an optional name= argument if you want to override the automatic name generation. But I'm happy with the matrix-removed names. I agree that it would be better if matrix.foo were then mentioned in the docstring of foo_matrix, feel free to fix this. But its not really necessary. I'm giving positive review to Robert's patch ;-)

Note: See TracTickets for help on using tickets.