Fix `IsIntegralRing` to return `false` for rings that are euclidean but not integral by changing `IsUniqueFactorizationRing` and `IsEuclideanRing` to not imply `IsIntegralRing` anymore #5817
This is an updated version of PR #3976 by @olexandr-konovalov -- I tried to update that PR but could not, so I opened a new one.
This fixes #3975 and closes #3976.
The main change is that I did not drop the implication
InstallTrueMethod(IsEuclideanRing, IsZmodnZObjNonprimeCollection and
IsWholeFamily and IsRing);
Instead I changed IsUniqueFactorizationRing (and hence IsEuclideanRing) to no longer imply IsIntegral. I also adjusted docstrings.
Just to avoid confusion, I'd like to point out that IsIntegral is a filter, so it never will give "correct" results in all cases -- it doesn't check the mathematical property but rather has a technical meaning. So IsIntegralRing(R) giving true should mean that the ring R is indeed an integral domain (only up to the existence of another bug, of course). While it giving false does not mean there are zero divisors, just that GAP cannot infer / prove that the ring is integral.
In the discussion on PR #3976, @hulpke suggested that if we do the changes that I now did here, then we should perhaps introduce new filters IsEuclideanDomain or IsEuclideanIntegralRing that have the old meaning (i.e. an alias for IsEuclideanRing and IsIntegralRing). In principle I could do that (though we probably should avoid the "Domain" variant -- while I like that name, unfortunately GAP chose to overload the term domain (and also category) with a meaning that differs from what it normally means in math, and I am concerned about this causing confusion). However I am not sure what I'd do with it? So far all methods I looked at and code I tried seem to work fine with the new meaning. So while I am not opposed, my suggestion would be to hold back on adding that until we find a concrete usecase? Of course if @hulpke or someone else already has something specific in mind right now, please let me know!
This is an updated version of PR #3976 by @olexandr-konovalov -- I tried to update that PR but could not, so I opened a new one.
This fixes #3975 and closes #3976.
The main change is that I did not drop the implication
Instead I changed
IsUniqueFactorizationRing(and henceIsEuclideanRing) to no longer implyIsIntegral. I also adjusted docstrings.Just to avoid confusion, I'd like to point out that
IsIntegralis a filter, so it never will give "correct" results in all cases -- it doesn't check the mathematical property but rather has a technical meaning. SoIsIntegralRing(R)givingtrueshould mean that the ringRis indeed an integral domain (only up to the existence of another bug, of course). While it givingfalsedoes not mean there are zero divisors, just that GAP cannot infer / prove that the ring is integral.In the discussion on PR #3976, @hulpke suggested that if we do the changes that I now did here, then we should perhaps introduce new filters
IsEuclideanDomainorIsEuclideanIntegralRingthat have the old meaning (i.e. an alias forIsEuclideanRing and IsIntegralRing). In principle I could do that (though we probably should avoid the "Domain" variant -- while I like that name, unfortunately GAP chose to overload the term domain (and also category) with a meaning that differs from what it normally means in math, and I am concerned about this causing confusion). However I am not sure what I'd do with it? So far all methods I looked at and code I tried seem to work fine with the new meaning. So while I am not opposed, my suggestion would be to hold back on adding that until we find a concrete usecase? Of course if @hulpke or someone else already has something specific in mind right now, please let me know!