Opened 8 years ago
Closed 8 years ago
#4601 closed defect (fixed)
[with patch; positive review] optional magma interface -- fix all broken optional doctests by introducing _magma_init_(self, magma) signature
Reported by: | was | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-3.2.1 |
Component: | interfaces | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket changes the signature of _magma_ and _magma_init_ slightly to pass in the interface -- which makes a MASSIVE amount of sense if you think about it. Also, the magma interface takes care of caching of values, which also makes sense.
There is some redundancy -- the generic infrastructure also separately caches stuff as well. This will be eliminated in a future ticket. The key point about this patch is that it applies cleanly to 3.2.1.alpha0 *and* it works -- it fixes all problems in trac's #4482, #4401 and #4399.
Attachments (2)
Change History (7)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Description modified (diff)
comment:2 Changed 8 years ago by
- Summary changed from [with patch; needs review] optional magma interface -- fix all broken optional doctests by introducing _magma_init_(self, magma) signature to [with patch; positive review] optional magma interface -- fix all broken optional doctests by introducing _magma_init_(self, magma) signature
There is one slight problem here with the doctests:
sage -t -long devel/sage/sage/interfaces/magma.py ********************************************************************** File "/scratch/mabshoff/release-cycle/sage-3.2.1.alpha1/devel/sage/sage/interfaces/magma.py", line 1559: sage: magma(S.gen_names()[1]) Exception raised: Traceback (most recent call last): File "/scratch/mabshoff/release-cycle/sage-3.2.1.alpha1/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/scratch/mabshoff/release-cycle/sage-3.2.1.alpha1/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/scratch/mabshoff/release-cycle/sage-3.2.1.alpha1/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_52[3]>", line 1, in <module> magma(S.gen_names()[Integer(1)])###line 1559: sage: magma(S.gen_names()[1]) NameError: name 'S' is not defined **********************************************************************
But that is obviously easy to fix :)
I have read the patch and while it wouldn't hurt that mhansen for example would take another look everything looks peachy :)
Cheers,
Michael
comment:3 Changed 8 years ago by
Sorry, I forgot a # optional - magma. I assume you're going to fix that... or are you requesting I do it?
comment:4 Changed 8 years ago by
I am fixing it and I will also put a dummy Magma executable in $PATH before the real one to make sure no missing "#optional" slips by :)
Cheers,
Michael
comment:5 Changed 8 years ago by
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 3.2.1.alpha1 - I will put a reviewers patch for the doctest issue in a second for completeness sake.
Cheers,
Michael
this patch should be applied to sage-3.2.1.alpha0. it should fix *all* optional doctest failures related to the magma interface! Note that there is a problem with -only_optional=magma (#4600) so doctesting with that will still show a few false errors.