I agree that there are cases that shouldn't be changed to an
ArithmeticError, and that grepping for "invertible" isn't sufficient. But
I think with a narrower scope this change is a good idea: if the error
arises from attempting to invert a non-invertible element of a ring.
So +1 from me, with a manual check that the change seems appropriate.
David
On Mon, Feb 5, 2024 at 7:54 PM Dave Morris <dmor...@deductivepress.ca>
wrote:
> -1 from me.
>
> Looking at just a few uncovered some that I think are definitely not
> ArithmeticError.
>
> Examples:
> There is an occurrence of ValueError('{} is not invertible') in
> the lift_isometry method of cliffordalgebra.py. Lifting an isometry is
> clearly not an arithmetic operation. ValueError is correct here.
> There is an occurrence of TypeError('the A-basis is defined only
> when 2 is invertible') in the method to find an `A`-basis of an
> Iwahori-Hecke algebra. Finding such a basis is clearly not an arithmetic
> operation. Perhaps this should be a ValueError instead of a TypeError,
> though.
> RuntimeError("morphism is not invertible"). I think that finding
> the inverse of a morphism between two objects of a category is clearly not
> an arithmetic operation. Perhaps this should be a ValueError rather than a
> RuntimeError.
>
> If you want to unify, I think ValueError might work. But I think they all
> need to be inspected, not just assume a single error type fits.
>
> > On Feb 5, 2024, at 4:44 AM, 'Martin R' via sage-devel <
> sage-devel@googlegroups.com> wrote:
> >
> > Dear all,
> >
> > currently, when trying to invert a non-invertible element, one of the
> following errors is raised (found using `grep -r --include=*.{py,pyx}
> --color -nH --null -e "Error(.*invertible" *`):
> > • 21 ValueError('{} is not invertible')
> > • 11 ZeroDivisionError("element is not invertible")
> > • 10 TypeError('the A-basis is defined only when 2 is invertible')
> > • 8 ArithmeticError("self must be invertible to have a
> multiplicative order")
> > • 2 RuntimeError("morphism is not invertible")
> > • 2 NotImplementedError("matrix must be invertible")
> >
> > Travis and I would like to propose to unify these to "ArithmeticError".
> If nobody objects, I would prepare a PR within the next few days.
> >
> > Best wishes,
> >
> > Martin
> >
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "sage-devel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to sage-devel+unsubscr...@googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/sage-devel/d0637584-4908-4101-8e1b-74ec0477ff84n%40googlegroups.com
> .
>
> --
> You received this message because you are subscribed to the Google Groups
> "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to sage-devel+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/sage-devel/4A0E1F77-27A7-4DF3-BB18-636853BF4CC0%40deductivepress.ca
> .
>
--
You received this message because you are subscribed to the Google Groups
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/sage-devel/CAChs6_maKC%3DFUF63uZNwk7DNaYfwa0S0pxEc-cp6CrfmCcQ7dw%40mail.gmail.com.
