#6329 closed defect (fixed)
optional doctest failure -- breakage in the sage<-->magma interface
Reported by: | was | Owned by: | tbd |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.7.2 |
Component: | interfaces | Keywords: | sd32 |
Cc: | Merged in: | sage-4.7.2.alpha3 | |
Authors: | Mariah Lenox, William Stein | Reviewers: | Mike Hansen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
sage -t -long --optional devel/sage/sage/rings/rational.pyx ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/rings/rational.pyx", line 3087: sage: magma(3/1).Type() # optional Expected: FldRatElt Got: RngIntElt ********************************************************************** 1 items had failures: 1 of 5 in __main__.example_84 ***Test Failed*** 1 failures.
Apply only trac_6329.2.patch to the Sage library.
Attachments (2)
Change History (16)
comment:1 Changed 12 years ago by
- Summary changed from optional doctest failure -- bad data type breakage in the sage-->magma interface to optional doctest failure -- breakage in the sage<-->magma interface
comment:2 Changed 12 years ago by
More failures:
sage -t -long --optional devel/sage/sage/interfaces/magma.py ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 856: sage: magma.attach('%s/data/extcode/magma/sage/basic.m'%Sage_ROOT) # optional - magma Exception raised: Traceback (most recent call last): File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_20[2]>", line 1, in <module> magma.attach('%s/data/extcode/magma/sage/basic.m'%Sage_ROOT) # optional - magma###line 856: sage: magma.attach('%s/data/extcode/magma/sage/basic.m'%Sage_ROOT) # optional - magma NameError: name 'Sage_ROOT' is not defined ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 860: sage: magma.attach('%s/data/extcode/magma/sage/basic2.m'%Sage_ROOT) # optional - magma Expected: Traceback (most recent call last): ... RuntimeError: Error evaluating Magma code... Got: Traceback (most recent call last): File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/scratch/wstein/build/sage-4.0.2.alpha3/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_20[3]>", line 1, in <module> magma.attach('%s/data/extcode/magma/sage/basic2.m'%Sage_ROOT) # optional - magma###line 860: sage: magma.attach('%s/data/extcode/magma/sage/basic2.m'%Sage_ROOT) # optional - magma NameError: name 'Sage_ROOT' is not defined ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 917: sage: print magma.load(SAGE_TMP + 'a.m') # optional - magma Expected: Loading ".../.sage//temp/.../a.m" hi Got: Loading "/scratch/wstein/sage//temp/sage.math.washington.edu/31604/a.m" hi ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 2148: sage: magma.eval('R<x> := PolynomialRing(RationalField()); f := (x-17/2)^3;') # optional - magma Expected: " Got: '' ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 2160: sage: magma.eval('K<a> := CyclotomicField(11)') # optional - magma Expected: " Got: '' ********************************************************************** File "/scratch/wstein/build/sage-4.0.2.alpha3/devel/sage-main/sage/interfaces/magma.py", line 463: sage: magma.eval("a := %s;"%(10^10000)) # optional - magma Expected: " Got: '' ********************************************************************** 4 items had failures: 2 of 4 in __main__.example_20 1 of 5 in __main__.example_22 2 of 28 in __main__.example_64 1 of 4 in __main__.example_9 ***Test Failed*** 6 failures. For whitespace errors, see the file /home/wstein/build/sage-4.0.2.alpha3/tmp/.doctest_magma.py [32.9 s]
Changed 10 years ago by
comment:3 Changed 10 years ago by
- Milestone changed from sage-4.7 to sage-4.7.1
- Priority changed from major to minor
- Report Upstream set to N/A
- Status changed from new to needs_review
trac_6329.patch corrects the test output in the problem in the description. The Got output is correct - there is not a breakage in the sage<-->magma interface. 'magma(3/1)' returns the integer '3' (after coercion), so Type() returns that the value is an integer.
The other two reports of failures are no longer valid, as they seem to already be fixed in sage-4.7.rc4.
comment:4 Changed 10 years ago by
- Component changed from optional packages to interfaces
comment:5 Changed 10 years ago by
- Status changed from needs_review to needs_info
It seems like the correct fix would be to always put the denominator in the _magma_init_ method.
s = self.numerator()._magma_init_(magma) s += '/' + self.denominator()._magma_init_(magma) return s
instead of
s = self.numerator()._magma_init_(magma) if not self.is_integral(): s += '/' + self.denominator()._magma_init_(magma) return s
comment:6 Changed 10 years ago by
Mhansen, you're right, the patch Mariah posted is wrong. I'll post a correct patch, and hopefully you can review it.
comment:7 Changed 10 years ago by
- Status changed from needs_info to needs_review
Patch up that is ready for review (the .2 one). This was a result of some overzealous optimization on my part a few years ago. Reverting this change will barely slow things down.
comment:8 follow-up: ↓ 9 Changed 10 years ago by
We should add a doctest for the integral case.
comment:9 in reply to: ↑ 8 Changed 10 years ago by
Replying to mhansen:
We should add a doctest for the integral case.
What do you mean by this? If you mean sage: magma(3/1).Type()
, which is in the ticket description, then there is *already* a doctest for this case, which is how we found this bug in the first place.
Or do you mean adding something to integer.pyx???
comment:10 Changed 10 years ago by
- Keywords sd32 added
comment:11 Changed 10 years ago by
- Status changed from needs_review to positive_review
I was thinking about the old code and verifying that the "is_integral" code path worked (since that test was missing before). However, it is obviously not needed now.
Positive review.
comment:12 Changed 10 years ago by
- Description modified (diff)
- Reviewers set to Mike Hansen
comment:13 Changed 10 years ago by
- Merged in set to sage-4.7.2.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
comment:14 Changed 9 years ago by
Shame on the idiot who wrote this with no tests... and who forgot that there are optional tests all over Sage that would be broken by this: see #12006, where I'm doing my penance.
More failures: