Opened 11 years ago

Closed 7 years ago

#9321 closed defect (fixed)

Documentation for sum() function should indicate Python syntax *first*

Reported by: rlm Owned by: mvngu
Priority: critical Milestone:
Component: documentation Keywords:
Cc: kcrisman Merged in:
Authors: Ralf Stephan Reviewers: Karl-Dieter Crisman, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 5f47daf (Commits, GitHub, GitLab) Commit: 5f47daf7c25df7063b612899d363a1a7f6719895
Dependencies: Stopgaps:

Status badges

Description

When did we hijack the sum function? Based on the documentation there, I have (today alone) had four different people come up to me and ask why something like the following doesn't work:

sage: sum(Integer(x), x, 0, 9)

I know the reasons this shouldn't work, but newbies definitely don't. It should say something about how to do

sage: sum( Integer(x) for x in range(10) )

before "getting all symbolic."

Change History (18)

comment:1 Changed 10 years ago by kcrisman

  • Cc kcrisman added

comment:2 Changed 7 years ago by rws

  • Priority changed from major to critical

The sagenb bug spreadsheet has several examples, too.

comment:3 Changed 7 years ago by rws

  • Branch set to u/rws/ticket/9321
  • Modified changed from 03/18/14 14:51:45 to 03/18/14 14:51:45

comment:4 Changed 7 years ago by rws

  • Commit set to 228fd67db789406492dce289ee437bb5a04f1333
  • Status changed from new to needs_review

New commits:

228fd67Trac #9321: add warnings to sum() and symbolic_sum() documentation

comment:5 Changed 7 years ago by rws

  • Authors set to Ralf Stephan

comment:6 Changed 7 years ago by kcrisman

Nice first step. I would also add some actual examples as the original reporter suggests - maybe with an explicit example showing what does and doesn't work along these lines.

comment:7 Changed 7 years ago by git

  • Commit changed from 228fd67db789406492dce289ee437bb5a04f1333 to 09ba9b9e6c3868d4f8221afbd8940012967f79cc

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

09ba9b9examples when/not to use Sage sum(); same also in calculus.py

comment:8 Changed 7 years ago by rws

How about this? Cannot make it any shorter than that.

comment:9 Changed 7 years ago by git

  • Commit changed from 09ba9b9e6c3868d4f8221afbd8940012967f79cc to 45fbd5044269755455b80d095f6e52e8dd7b9fe2

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

45fbd50fix doctests

comment:10 Changed 7 years ago by tscrim

Your warning messages are indented one too many times.

comment:11 Changed 7 years ago by git

  • Commit changed from 45fbd5044269755455b80d095f6e52e8dd7b9fe2 to eb0ddc0a0269fe7e26ced69b162000b1b9aa1a6b

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

eb0ddc0too much indentation

comment:12 Changed 7 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman, Travis Scrimshaw

comment:13 Changed 7 years ago by rws

Thanks for the review.

comment:14 Changed 7 years ago by kcrisman

Okay, I'm going to ask for one final thing. Probably it isn't appropriate to have this warning be before one even sees the INPUT block! Can you put this after the first few examples, and then have some wording indicating "now back to the examples" in a not-informal way? Otherwise we'll have the opposite problem of everyone avoiding this function :)

Also, another nit-pick - try to put the sum() in double back ticks so that it typesets properly as code. And... is there any general reference for the Python sum, or are all of them version-dependent? (I think the latter, just checking in case you know).

Thanks! Sorry this is an incremental review but it will be more awesomer soon.

comment:15 Changed 7 years ago by rws

  • Branch changed from u/rws/ticket/9321 to u/rws/new9321
  • Commit changed from eb0ddc0a0269fe7e26ced69b162000b1b9aa1a6b to 5f47daf7c25df7063b612899d363a1a7f6719895

I moved the warning a bit lower and added necessary backticks. I also removed the version string from the python link, although you will still arrive at the 2.x version. It's no longer hardcoded however. Finally, I had to change the branch path because git amended commits are not accepted by sage -dev push.


New commits:

5f47dafmoved warning after some examples

comment:16 Changed 7 years ago by tscrim

Karl-Dieter, are you happy with the current version? (Really this is an elaborate ping.)

comment:17 Changed 7 years ago by kcrisman

  • Status changed from needs_review to positive_review

I also removed the version string from the python link, although you will still arrive at the 2.x version. It's no longer hardcoded however.

https://docs.python.org/{2,3}/library/functions.html#sum is the link, technically. I won't hold it up for that, though, since they can just click on "sum" from the big list at that location. Doc looks good now.

Karl-Dieter, are you happy with the current version? (Really this is an elaborate ping.)

:-) Sorry for the delay; I definitely have been having to cut back even on review time the past few months.

comment:18 Changed 7 years ago by vbraun

  • Branch changed from u/rws/new9321 to 5f47daf7c25df7063b612899d363a1a7f6719895
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.