#5631 closed enhancement (fixed)
[with patch, positive review] improve doctest coverage for schemes/generic/affine_space.py
Reported by: | AlexGhitza | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | minor | Milestone: | sage-3.4.1 |
Component: | algebraic geometry | Keywords: | doctest affine space |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The attached patch improves the doctest coverage of affine_space.py
from 45% (9 of 20) to 80% (16 of 20) and fixes a few tiny bugs along the way.
Attachments (2)
Change History (16)
comment:1 Changed 14 years ago by
Milestone: | → sage-3.4.1 |
---|---|
Summary: | improve doctest coverage for schemes/generic/affine_space.py → [with patch, needs review] improve doctest coverage for schemes/generic/affine_space.py |
comment:2 Changed 14 years ago by
Owner: | changed from was to AlexGhitza |
---|
comment:3 Changed 14 years ago by
Status: | new → assigned |
---|
comment:4 Changed 14 years ago by
Summary: | [with patch, needs review] improve doctest coverage for schemes/generic/affine_space.py → [with patch, needs work] improve doctest coverage for schemes/generic/affine_space.py |
---|
comment:5 follow-up: 6 Changed 14 years ago by
I had two problems with lines 415 and 416: (1) the doctest that I wrote wasn't working and (2) they aren't there in the corresponding method in projective_space.py
. If you look at the docstring that I wrote for subscheme_complement
(and the doctest), you will notice that X and Y are already subschemes of self (so self is involved in a slightly hidden way).
I guess that one could relax the syntax so that polynomial lists can be passed to subscheme_complement
; in the example of the doctest this would be
sage: A.<x, y, z> = AffineSpace(3, ZZ) sage: A.subscheme_complement([x+y-z], [x-y+z])
Is this what you have in mind?
comment:6 Changed 14 years ago by
If you look at the docstring that I wrote for subscheme_complement (and the doctest), you will notice that X and Y are already subschemes of self (so self is involved in a slightly hidden way).
Couldn't I do this:
sage: A.<x, y, z> = AffineSpace(3, ZZ) sage: [[make an X and a Y over GF(7) say that haven't nothing to do with A]] sage: A.subscheme_complement(X, Y)
Either subscheme_complement needs to check that X and Y are really subschemes of A, or it should just be a method of X, i.e.,
sage: X.complement(Y)
say, which requires that X and Y live in a common ambient space.
-- William
comment:7 Changed 14 years ago by
So I looked at algebraic_scheme.py and noticed that there is a method X.exclude(Y)
which seems to want to do what you're suggesting. It's actually broken as it is, and I doubt that it ever worked (no doctests, not used anywhere in the Sage library, and the simplest tests that I thought of didn't work). So I'm renaming it to X.complement(Y)
and removing subscheme_complement()
from both affine_space.py
and projective_space.py
. They are also not used anywhere.
Changed 14 years ago by
Attachment: | trac_5631.patch added |
---|
comment:8 Changed 14 years ago by
Summary: | [with patch, needs work] improve doctest coverage for schemes/generic/affine_space.py → [with patch, needs review] improve doctest coverage for schemes/generic/affine_space.py |
---|
comment:9 Changed 14 years ago by
Summary: | [with patch, needs review] improve doctest coverage for schemes/generic/affine_space.py → [with patch, needs work] improve doctest coverage for schemes/generic/affine_space.py |
---|
Can you rebase it? Or does it depends on something? I can't apply it:
teragon:build wstein$ sage ---------------------------------------------------------------------- | Sage Version 3.4.1.rc1, Release Date: 2009-04-05 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- hg_sage.apply('http://trac.sagemath.org/sage_trac/raw-attachment/ticket/5631/trac_5631.patch') sage: hg_sage.apply('http://trac.sagemath.org/sage_trac/raw-attachment/ticket/5631/trac_5631.patch') Attempting to load remote file: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/5631/trac_5631.patch Loading: [..] cd "/Users/wstein/build/sage-3.4.1.rc1/devel/sage" && hg status cd "/Users/wstein/build/sage-3.4.1.rc1/devel/sage" && hg status cd "/Users/wstein/build/sage-3.4.1.rc1/devel/sage" && hg import "/Users/wstein/.sage/temp/teragon.local/86960/tmp_1.patch" applying /Users/wstein/.sage/temp/teragon.local/86960/tmp_1.patch patching file sage/schemes/generic/affine_space.py Hunk #4 FAILED at 208 Hunk #5 succeeded at 243 with fuzz 2 (offset 0 lines). Hunk #6 FAILED at 264 2 out of 8 hunks FAILED -- saving rejects to file sage/schemes/generic/affine_space.py.rej abort: patch failed to apply sage:
Changed 14 years ago by
Attachment: | trac_5631_rebased.patch added |
---|
apply instead of the previous patch
comment:10 Changed 14 years ago by
Yes, there was some interaction with the ticket that fixed dimension issues.
The new patch applies to 3.4.1.rc1.
comment:11 Changed 14 years ago by
Summary: | [with patch, needs work] improve doctest coverage for schemes/generic/affine_space.py → [with patch, needs review] improve doctest coverage for schemes/generic/affine_space.py |
---|
comment:12 Changed 14 years ago by
Summary: | [with patch, needs review] improve doctest coverage for schemes/generic/affine_space.py → [with patch, positive review] improve doctest coverage for schemes/generic/affine_space.py |
---|
comment:13 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Merged trac_5631_rebased.patch in Sage 3.4.1.rc3.
Cheers,
Michael
comment:14 Changed 14 years ago by
Milestone: | sage-3.4.2 → sage-3.4.1 |
---|
I don't understand why you deleted lines 415, 416 below:
The way you have the code now, it subscheme_complement has nothing at all to do with self. Why is it even a method of self as is now.
Otherwise this patch looks very good.