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 )
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)
Change History (11)
comment:1 Changed 9 years ago by
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
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
- 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
comment:4 Changed 9 years ago by
- 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
comment:5 Changed 9 years ago by
I had to rebase this, for some reason the patch wouldn't apply properly.
Changed 9 years ago by
comment:6 Changed 9 years ago by
- 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
- 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
- Merged in set to sage-5.11.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
this is my first patch! For anyone looking at it please help me become a better patcher!
thank you :)
Nathaniel Skinner