Errors should be raised, not returned.

[simon-king-jena]

The following issue was considered in at least two threads, the latest at http://groups.google.com/group/sage-devel/browse_thread/thread/3661fde739474fdb.

There are several places in the Sage code where errors are not raised but returned. A hopefully exhaustive search brought up the following:

sage: import exceptions
sage: for E in dir(exceptions):
....:     if not E.startswith('__'):
....:         s = search_src("return "+E)
....:         if s:
....:             print s
....:
rings/finite_field_element.py:384:                return ArithmeticError, "Multiplicative order of 0 not defined."
rings/finite_field_givaro.pyx:1956:                return ArithmeticError, "Multiplicative order of 0 not defined."
structure/element.pyx:2601:            return ArithmeticError, "Multiplicative order of 0 not defined."

rings/ring.pyx:687:            return NotImplementedError
modular/hecke/module.py:706:        abstract base class, return NotImplementedError.
modular/arithgroup/congroup_gammaH.py:928:            return NotImplementedError
geometry/polyhedra.py:1068:            return NotImplementedError
symbolic/expression.pyx:1524:        return NotImplementedError
symbolic/expression_conversions.py:638:            return NotImplementedError("SymPy function '%s' doesn't exist" % f)

interfaces/gap.py:580:            return RuntimeError, "Error evaluating %s in %s"%(line, self)

modular/abvar/finite_subgroup.py:280:            return ValueError, "self and other must be in the same ambient Jacobian"
groups/perm_gps/permgroup_named.py:945:            return ValueError, "Degree must be 2."
groups/perm_gps/permgroup_named.py:988:            return ValueError, "Degree must be 2."

Of course, if an error is returned it can't be catched, which implies obvious problems.

I have no idea what component that ticket should be associated with. "Performance" seemed the least inappropriate description to me.

Is there any of the above cases in which the error should really be returned, not raised?

Component: misc

Keywords: error return

Author: Tim Dumol

Reviewer: John Palmieri

Merged: sage-4.3.2.alpha0

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

[jhpalmieri]

comment:2

See also #7532.

[TimDumol]

comment:4

This should do the trick.

[TimDumol]

Makes all remaining returns of exceptions into raising.

[jhpalmieri]

comment:5

Attachment: trac_7535-errors-raise.patch.gz

I'm not sure what you mean by "remaining", since there is no patch at #7532 (or elsewhere?) fixing any other instances of this. I'm attaching a patch dealing with two more of these, leaving, I think, just the one in rings.pyx. See #7532 for that one.

Positive review for timdumol's patch, so if mine is okay, this whole ticket can get a positive review.

[jhpalmieri]

Reviewer: John Palmieri

[jhpalmieri]

Author: Tim Dumol

[jhpalmieri]

apply on top of the other patch

[JohnCremona]

comment:6

Attachment: trac_7535-part2.patch.gz

I am about to add a patch to #7532 which fixes (for me) the remaining issue in schemes/elliptic_curves.

[TimDumol]

comment:7

Doctests pass with the latter patch and the one in #7532, so I'll mark both as positive review.

[sagetrac-mvngu]

Merged: sage-4.3.2.alpha0

[sagetrac-mvngu]

comment:8

Merged in this order:

1. trac_7535-errors-raise.patch --- timdumol: please remember to put your username in your patch. I have merged this patch in your name.
2. trac_7535-part2.patch