Opened 9 years ago

Closed 9 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 was)

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)

sage-4601.patch (56.1 KB) - added by was 9 years ago.
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.
trac_4601_reviewer.patch (1.6 KB) - added by mabshoff 9 years ago.

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by was

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.

comment:1 Changed 9 years ago by was

  • Description modified (diff)

comment:2 Changed 9 years ago by mabshoff

  • 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 9 years ago by was

Sorry, I forgot a # optional - magma. I assume you're going to fix that... or are you requesting I do it?

comment:4 Changed 9 years ago by mabshoff

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 9 years ago by mabshoff

  • 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

Changed 9 years ago by mabshoff

Note: See TracTickets for help on using tickets.