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

improve doctest coverage for schemes/generic/spec.py #5630

Closed
aghitza opened this issue Mar 29, 2009 · 3 comments
Closed

improve doctest coverage for schemes/generic/spec.py #5630

aghitza opened this issue Mar 29, 2009 · 3 comments

Comments

@aghitza
Copy link

aghitza commented Mar 29, 2009

The attached patch adds a _latex_() method for Spec's of rings and improves the doctest coverage of spec.py from 42% (3 of 7) to 75% (6 of 8).

The two remaining methods are currently involved in other tickets that will also take care of adding doctests: see #5629 for dimension() and #5479 for __call__()

Component: algebraic geometry

Keywords: spec doctest latex

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

@williamstein
Copy link
Contributor

comment:4

This doctest fails for me on 32-bit OS X:

teragon:sage-3.4 wstein$ sage -t devel/sage/sage/schemes/generic/spec.py 
sage -t  "devel/sage/sage/schemes/generic/spec.py"          
**********************************************************************
File "/Users/wstein/build/sage-3.4/devel/sage/sage/schemes/generic/spec.py", line 116:
    sage: Spec(QQ) < 5
Expected:
    True
Got:
    False

Since the result is meaningless, you could flag it

sage:  spec(QQ) < 5   # random -- platform dependent

or instead have a test

sage: spec(QQ) == 5
False

If you fix this one issue, then this will get "positive review" from me.

@aghitza
Copy link
Author

aghitza commented Mar 29, 2009

comment:5

Attachment: trac_5630.patch.gz

Ah, I had misinterpreted the existing docstring for _cmp_. I removed the offending doctest (the one with Spec(QQ) == 5 is already there) and clarified the docstring a little bit.

New patch is up replacing the old one.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 31, 2009

comment:7

Merged in Sage 3.4.1.rc0.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Mar 31, 2009
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

2 participants