Opened 10 years ago

Closed 10 years ago

#10362 closed defect (fixed)

Improve vector constructor documentation

Reported by: rbeezer Owned by: jason, was
Priority: major Milestone: sage-4.6.1
Component: linear algebra Keywords:
Cc: jason Merged in: sage-4.6.1.alpha3
Authors: Rob Beezer Reviewers: Andrey Novoseltsev
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by rbeezer)

This is a documentation-only patch, other than some minor whitespace clean-up in the code. It brings the documentation of the vector() constructor in line with the code. Specifically the handling of integer arguments (degree), the absence of entries (creating zero vectors), and some subtleties with dictionary input.

It suggests some code changes: negative integer arguments, more care with dictionary input, and a new zero_vector() constructor, which will go on another ticket (see #10364).

Attachments (2)

trac_10362-vector-constructor-documentation.patch (7.1 KB) - added by rbeezer 10 years ago.
trac_10362_reviewer.patch (1.4 KB) - added by novoselt 10 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by rbeezer

  • Description modified (diff)

comment:2 Changed 10 years ago by rbeezer

  • Status changed from new to needs_review

comment:3 Changed 10 years ago by jason

  • Cc jason added

Changed 10 years ago by novoselt

comment:4 Changed 10 years ago by novoselt

  • Milestone set to sage-4.6.1
  • Reviewers set to Andrey Novoseltsev

Looks like a nice improvement, except that dictionary-related parts seem to be weird, but this is addressed by another ticket.

In the original patch "INPUT" is "glued" to the numbered list above, so I indented the list to separate them. Also, I like when "OUTPUT" block looks the same as "INPUT" and in this particular case it really makes sense, I think, because the output description is so long. So I have added a blank line as well.

If you agree with the changes, please switch to positive review!

comment:5 Changed 10 years ago by rbeezer

  • Status changed from needs_review to positive_review

Andrey,

Thanks for the review and fixes. I'd noticed the "gluing" but had not experimented with a fix. I think I have a couple more tickets in-progress where I need to make that change as well.

I'm conflicted on the format of the OUTPUT block header and vacillate between the two options. Your change is fine.

Reviewer patch applies, tests, docbuilds just fine. So I'll flip this to positive review.

Thanks again, Rob

Release Manger

Apply original patch and then reviewer patch.

comment:6 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.