#15588: Cleanup of integer_mod_ring.py
-------------------------------------+-------------------------------------
Reporter: tscrim | Owner: tscrim
Type: enhancement | Status: positive_review
Priority: major | Milestone: sage-6.1
Component: doctest coverage | Resolution:
Keywords: | Merged in:
Authors: Travis Scrimshaw | Reviewers: Nathann Cohen
Report Upstream: N/A | Work issues:
Branch: | Commit:
public/rings/cleanup_integer_mod_ring|
0e1a46b94e5094921fe35561e75c9369d7cd7d92
Dependencies: #15369 | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by tscrim):
Replying to [comment:2 ncohen]:
> Even though we don't agree on style issues. It looks like you prefer to
have many "if/return, if/return,if/return" while I prefer "elif/return",
and for some reason you don't like "else" at all.
I just get annoyed when there's something like:
{{{
if cond:
special/small case
return foo
else:
a long code block
return foobar
}}}
and I just try to be consistent with that overall. Plus the extra indents
can play hell on my sanity when going from 5 indents to 1 (or was it 6 or
4... `:P`). Okay, I'll stop ranting.
> Ahahaah. Anyway, it's good to go, thank you for that !
Thank you for doing the review!
> And I have to read this awful thing soon, for I can't go on having to
complain each time the categories are involved.
>
http://www.sagemath.org/doc/reference/categories/sage/categories/primer.html
I think Simon's "implementing a new parent" (or something like that)
tutorial is a better starting point.
> Besides, this example for your docstring is reaaaaaaaallllyyy scary :
It has to do with refining the category and doing primality checks that
one may not want todo when constructing the object. There's a topic on
sage-devel and a ticket on this as I recall. For example, do you know if
`ZZ / 10000000000000069 ZZ` is a field? It is in fact, and now that you do
know it, you can do things like you would with other fields. That's
basically what that docstring is about.
> Anyway. I tried to pay very close attention, and all I could spot in
your commit is that I would have written {{{`GF(p) -> Z/pZ`}}} instead of
{{{ ``GF(p) -> Z/pZ``}}}. Which I guess is only style again `:-P`
I just made it verbatim, but since it's not seen in the built doc (in
fact, almost noone will ever see that), I didn't put much thought into it.
Feel free to change it.
Thanks again Nathann,[[BR]]
Travis
--
Ticket URL: <http://trac.sagemath.org/ticket/15588#comment:3>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
