Add quickstarts from PREP workshops to standard documentation

kcrisman commented 12 years ago

This ticket is to add the quickstart tutorials, originally at #13260. It was split off from that ticket for convenience.

Apply attachment: trac_13381-quickstarts.patch, attachment: trac_13381-ref.patch, and attachment: trac_13381-fixtests.patch.

Depends on #5457 Depends on #9005 Depends on #14014

CC: @jasongrout @rbeezer @dandrake @wdjoyner @sagetrac-mhampton @benjaminfjones @wypong @dcernst @sagetrac-travis @novoselt @jhpalmieri

Component: documentation

Author: Karl-Dieter Crisman

Reviewer: John Palmieri, Jason Grout, Jeroen Demeyer

Merged: sage-5.11.rc0

Issue created by migration from https://trac.sagemath.org/ticket/13381

kcrisman commented 12 years ago

Dependencies: #13260

kcrisman commented 12 years ago

Based on 5.3.beta1

kcrisman commented 12 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
 This ticket is to add the quickstart tutorials, originally at #13260.  It was split off from that ticket for convenience.
+
+Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz).

kcrisman commented 12 years ago

comment:2

Attachment: trac_13381-quickstarts.patch.gz

Ready for review.

Patchbot, apply trac_13381-quickstarts.patch.

jhpalmieri commented 12 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
 This ticket is to add the quickstart tutorials, originally at #13260.  It was split off from that ticket for convenience.

-Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz).
+Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz) and [attachment: trac_13381-ref.patch](https://github.com/sagemath/sage-prod/files/10656210/trac_13381-ref.patch.gz).

jhpalmieri commented 12 years ago

comment:3

Here is a referee patch. If it's okay with you, then one item remains to be addressed: in NumAnalysis.rst:

"The first step is to normalize" - The first step in what? It's not clear what's being done here. It looks like you want to convert x to floating point binary, but it also looks like that was done a few lines earlier in x.str(base=2). So a little more explanation would be helpful.

Other comments (I think most of my changes are self-explanatory):

in Abstract-Algebra.rst and elsewhere, I've changed \ZZ_n to \ZZ/n\ZZ. The subscript notation is ambiguous — many people will read \ZZ_p as denoting the p-adics, not the integers mod p, and so in my opinion this notation should never be used to denote quotient rings of the integers.
in Linear-Algebra.rst, I've added a definition of "left kernel": personally, I can tell from the name "left kernel" that either the matrix or the vector is supposed to be on the left, but I can never remember which.

The use of upper case in the file names here (and in the parent directory) doesn't follow the same pattern as elsewhere in the Sage library. This is certainly not a big deal, but it looks a little strange to me.

jhpalmieri commented 12 years ago

Attachment: trac_13381-ref.patch.gz

referee patch; apply on top of other one

jasongrout commented 12 years ago

comment:4

regarding left vs. right: I guess I always think that the way I do it naturally is the "right" (correct) way to do it :). That helps me remember that the normal linear algebra way (column vectors, A*x) is the "right" way.

Not that I'm biased or anything :).

jasongrout commented 12 years ago

comment:5

John, did you review this positively, or is there still more to do?

jhpalmieri commented 12 years ago

comment:6

In my comment above, I said that "one item remains to be addressed". This is still true.

kcrisman commented 12 years ago

comment:7

I think he was at least thinking the NumAnalysis.rst thing about normalization needed to be fixed (which I think I agree about, but it was a while ago that I read this so I forget). Also, his patch is long enough that is probably could use a quick once-over to make sure he didn't miss any obvious formatting things, though this IS jhpalmieri, the king of Sphinx, we're talking about here :)

jasongrout commented 12 years ago

comment:8

The heading could be changed to "Converting to IEEE floating point binary format"---does that make things clearer? The rest of the paragraph in question explains that the first step in IEEE conversion is normalizing the number so that it is a number between 1 and 2 (multiplied by an appropriate power of 2).

jasongrout commented 12 years ago

comment:9

Or how about this rewording of the paragraph:

Now let's convert ``x`` to the IEEE 754 binary format that is commonly used in computers.  For `IEEE 754
<http://grouper.ieee.org/groups/754/>`_, the first step in getting the binary format is to normalize the 
number, or express the number as a number between 1 and 2 multiplied by a power of 2.  For our ``x`` above, 
we multiply by `2^{-3}` to get a number between 1 and 2.

Is that better?

kcrisman commented 12 years ago

Reviewer: John Palmieri, Jason Grout

kcrisman commented 12 years ago

comment:10

That seems reasonable. Actually, making the section heading "Converting to floating point binary (IEEE format)" would be the best compromise on the heading title as well.

jhpalmieri commented 12 years ago

comment:11

These all look like good suggestions to me.

kcrisman commented 11 years ago

Apply on top of others

kcrisman commented 11 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
 This ticket is to add the quickstart tutorials, originally at #13260.  It was split off from that ticket for convenience.

-Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz) and [attachment: trac_13381-ref.patch](https://github.com/sagemath/sage-prod/files/10656210/trac_13381-ref.patch.gz).
+Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz), [attachment: trac_13381-ref.patch](https://github.com/sagemath/sage-prod/files/10656210/trac_13381-ref.patch.gz), and [attachment: trac_13381-ieee.patch](https://github.com/sagemath/sage-prod/files/10656211/trac_13381-ieee.patch.gz).

kcrisman commented 11 years ago

Changed dependencies from #13260 to none

kcrisman commented 11 years ago

comment:12

Attachment: trac_13381-ieee.patch.gz

Okay, I've finished this off by just implementing this. John was okay with this, and all I did was put what Jason suggested. Yay!

jdemeyer commented 11 years ago

comment:13

There are obvious doctest failures, why did this ever get positive review?

While you're at it, you should also use the new-style doctest continuations

sage: for K in D.conjugacy_classes_subgroups(): 
....:     print K

instead of

sage: for K in D.conjugacy_classes_subgroups(): 
...       print K

jdemeyer commented 11 years ago

Dependencies: #5457, #9005, #14014

kcrisman commented 11 years ago

comment:14

There are obvious doctest failures, why did this ever get positive review?

Because the only previous complaints were about non-doctest issues. Patchbot is very flaky, in my opinion, and often fails for trivial reasons.

Interestingly, at least one of these failures indicates a regression in Sage. The rest are pretty easy to fix, I'll do this later today along with the new-style continuations.

kcrisman commented 11 years ago

comment:15

I also changed to new continuations in the main prep stuff. Very annoying. Some of the changes are pretty recent as well.

There are two remaining issues, I think.

sage: import scipy.stats
sage: my_data=[lognormvariate(2,3) for i in range(10)]
sage: scipy.stats.scoreatpercentile(my_data, int(50))
1.2439846750720158
sage: scipy.stats.scoreatpercentile(my_data, 50)
---------------------------------------------------------------------------

TypeError: 'sage.rings.integer.Integer' object is not iterable

This is because scipy now assumes a Sage Integer is not a scalar. I'm not sure whether this is an upstream bug or not.

Then there is this strange regression in plotting power series.

sage: R.<t> = PowerSeriesRing(QQ)
sage: c = t + 1 + O(t^2)
sage: plot(c,(t,0,1))
AttributeError: 'float' object has no attribute 'parent'

If I just do the polynomial, it's fine, so I can fix the test. Interestingly, I get a different error with the standard syntax:

plot(c.polynomial(),(t,0,1))
Error...

kcrisman commented 11 years ago

comment:16

I think I have discovered a really nasty bug with new-style line continuations. I cannot get live documentation to work with it. So I am not going to do this. For some reason the new such doctests don't seem to be in the live documentation (probably in underscore methods, and not a lot of them), so that's why it hasn't been noticed. This is as of 5.11.beta1.

kcrisman commented 11 years ago

Changed reviewer from John Palmieri, Jason Grout to John Palmieri, Jason Grout, Jeroen Demeyer

kcrisman commented 11 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
 This ticket is to add the quickstart tutorials, originally at #13260.  It was split off from that ticket for convenience.

-Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz), [attachment: trac_13381-ref.patch](https://github.com/sagemath/sage-prod/files/10656210/trac_13381-ref.patch.gz), and [attachment: trac_13381-ieee.patch](https://github.com/sagemath/sage-prod/files/10656211/trac_13381-ieee.patch.gz).
+Apply [attachment: trac_13381-quickstarts.patch](https://github.com/sagemath/sage-prod/files/10656209/trac_13381-quickstarts.patch.gz), [attachment: trac_13381-ref.patch](https://github.com/sagemath/sage-prod/files/10656210/trac_13381-ref.patch.gz), and [attachment: trac_13381-fixtests.patch](https://github.com/sagemath/sage-prod/files/10656212/trac_13381-fixtests.patch.gz).

kcrisman commented 11 years ago

comment:17

Patchbot, apply trac_13381-quickstarts.patch, trac_13381-ref.patch, and trac_13381-fixtests.patch

Doctests are now fixed, I fixed some broken links (no thanks to the reorganization of the reference manual!), and of course the IEEE business. All tests pass. There is nothing substantive changed, though, and I think jeroen's comments suffice.

kcrisman commented 11 years ago

comment:18

Still some issues in live documentation, one moment...

kcrisman commented 11 years ago

Apply last

kcrisman commented 11 years ago

comment:19

Attachment: trac_13381-fixtests.patch.gz

All set!!

Patchbot, apply trac_13381-quickstarts.patch, trac_13381-ref.patch, and trac_13381-fixtests.patch

jdemeyer commented 11 years ago

Merged: sage-5.11.rc0

sagemath / sage

Add quickstarts from PREP workshops to standard documentation #13381