Opened 9 years ago

Closed 9 years ago

#13857 closed enhancement (fixed)

Add symbolic max and min to reference manual

Reported by: kcrisman Owned by: mvngu
Priority: minor Milestone: sage-5.11
Component: documentation Keywords: beginner
Cc: Merged in: sage-5.11.rc1
Authors: Nathaniel Skinner Reviewers: Karl-Dieter Crisman, Nathann Cohen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by kcrisman)

In #6949, we added max_symbolic and min_symbolic to a new file, but it apparently isn't in the reference manual. Let's fix that; shouldn't be hard.

Apply trac_13857-rebased.patch and trac_13857-review.patch.

Attachments (3)

minmax.patch (2.3 KB) - added by Madrosity 9 years ago.
trac_13857-rebased.patch (2.3 KB) - added by kcrisman 9 years ago.
trac_13857-review.patch (1.1 KB) - added by kcrisman 9 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 9 years ago by Madrosity

  • Status changed from new to needs_review

this is my first patch! For anyone looking at it please help me become a better patcher!

thank you :)

Nathaniel Skinner

comment:2 Changed 9 years ago by kcrisman

  • Authors set to Nathaniel Skinner

Hi, and thanks for your first contribution! For now, I'll just point out that you can use

hg qrefresh -e

to change the commit message, which right now is the somewhat enigmatic [mq]: minmax. See the Sage developer manual for a similar command.

comment:3 Changed 9 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to needs_work

This works nicely. As noted above, you do need to change your commit message to something helpful (such as "added symbolic min and max to reference manual"). Nice catch on the typesetting of min and max.

I think this also needs a little bit of work because, although the examples in the symbolic min and max functions explain why there is this difference, the introductory material you added is sort of contextless; some people might only read this part. Giving a brief explanation of why these are necessary (perhaps along the lines already in the doc for the functions themselves) would be helpful. Adding one or two examples from the underscore methods may also be useful, you could look at them to see.

When you've done this, you can replace this patch when you upload it if you give the same name to the file (there is a checkbox for this). Then just set back to "needs review"!

Changed 9 years ago by Madrosity

comment:4 Changed 9 years ago by Madrosity

  • Status changed from needs_work to needs_review

round two of this patch hopefully it will work better this time! thanks for all the help

Nathaniel Skinner

Changed 9 years ago by kcrisman

comment:5 Changed 9 years ago by kcrisman

I had to rebase this, for some reason the patch wouldn't apply properly.

Changed 9 years ago by kcrisman

comment:6 Changed 9 years ago by kcrisman

  • Description modified (diff)

Okay, I added one additional example which I think would be useful. I also made some minor adjustments to wording. This does need some minor review just to make sure my additional example works and everything is fine.

Patchbot, apply trac_13857-rebased.patch and trac_13857-review.patch

comment:7 Changed 9 years ago by ncohen

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Nathann Cohen
  • Status changed from needs_review to positive_review

Gooooooooooooood to go :-)

Nathann

comment:8 Changed 9 years ago by jdemeyer

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