Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docstrings and doctests for rings/ideal.py #2305

Closed
cswiercz mannequin opened this issue Feb 25, 2008 · 7 comments
Closed

Docstrings and doctests for rings/ideal.py #2305

cswiercz mannequin opened this issue Feb 25, 2008 · 7 comments

Comments

@cswiercz
Copy link
Mannequin

cswiercz mannequin commented Feb 25, 2008

Provide missing docstrings and doctests for all non-"_" functions in rings/ideal.py. These include:

id_Ideal(x)
base_ring(self)
is_maximal(self)
is_prime(self)
is_principal(self)
is_principal(self)
gen(self)
gcd(self, other)

Component: documentation

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

@cswiercz cswiercz mannequin added this to the sage-2.10.3 milestone Feb 25, 2008
@cswiercz cswiercz mannequin self-assigned this Feb 25, 2008
@cswiercz
Copy link
Mannequin Author

cswiercz mannequin commented Feb 27, 2008

Attachment: ring.ideal.patch.gz

Remaining docstrings and doctetst for rings/ideal.py

@cswiercz cswiercz mannequin removed the s: needs review label Feb 27, 2008
@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Feb 28, 2008

comment:4

The patch is mostly fine, but there are some formatting issues.

This looks wrong:

 	155	        sage: R = ZZ
 	156	        sage: R; is_Ideal(R) 
 	157	        Integer Ring 
 	158	        False 

Please typeset sage's output all on one line. So rather than

 	168	        sage: I = R.ideal(x^2 + 1); I 
 	169	        Principal ideal (x^2 + 1) of Univariate Polynomial Ring in x over 
 	170	        Rational Field 

give me

 	168	        sage: I = R.ideal(x^2 + 1); I 
 	169	        Principal ideal (x^2 + 1) of Univariate Polynomial Ring in x over Rational Field 

Also, there are tons of typos. 'th' instead of 'the', incorrectly spelled words, etc. I will work on some emacs code that will spell-check only sage comments, and ignore examples as appropriate, but until then you'll have to do it by hand :)

@ncalexan ncalexan mannequin changed the title Docstrings and doctests for rings/ideal.py [with positive review pending reformatting and spellchecking] Docstrings and doctests for rings/ideal.py Feb 28, 2008
@cswiercz
Copy link
Mannequin Author

cswiercz mannequin commented Feb 28, 2008

Attachment: rings.ideal.patch.gz

Corrected docstring and doctest patch for rings/ideal.py

@cswiercz
Copy link
Mannequin Author

cswiercz mannequin commented Feb 28, 2008

comment:5

Accidentally renamed the patch. Be sure to review rings.ideal.patch and ignore ring.ideal.patch. Thanks!

@cswiercz cswiercz mannequin changed the title [with positive review pending reformatting and spellchecking] Docstrings and doctests for rings/ideal.py Docstrings and doctests for rings/ideal.py Feb 28, 2008
@cswiercz cswiercz mannequin added the s: needs review label Feb 28, 2008
@dfdeshom
Copy link

dfdeshom commented Mar 6, 2008

comment:6

Doctest-formatting looks good and typos are out. So I say apply!

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 7, 2008

comment:7

Merged rings.ideal.patch in 2.10.3.rc3. The patch does add a single docstring in numerical/optimize.py, so I am not quite sure if that was intended for this patch.

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Mar 7, 2008
@malb
Copy link
Member

malb commented Mar 7, 2008

comment:8

Sorry for being late to the party, but please open another ticket to address the following minor issues:

  • def ring the docstring uses backslashes but is not treated raw: r""" missing
  • "Note that \code{self.ring()} is different from \code{self.ring()}" seems like a typo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants