Problems with consistency in field element checks (finite fields)

[Nils Bruin]

I ran into the following worrisome example. In sage.rings.finite_rings.element_base we have that is_FiniteFieldElement(x) has a deprecation on it:

   deprecation(32664, "the function is_FiniteFieldElement is deprecated; use isinstance(x, sage.structure.element.FieldElement) and x.parent().is_finite() instead")

Yet:

sage: isinstance(GF(2).0, sage.structure.element.FieldElement)
False

how is it possible that something got deprecated in favour of something that doesn't even work in the most basic example?

We do have that parent(GF(2).0).is_field() is true and that it inherits from sage.rings.ring.Field, so on the parent side things seem to be OK.

[Dima Pasechnik]

On Mon, Apr 28, 2025 at 1:20 PM Nils Bruin <nbr...@sfu.ca> wrote:

I ran into the following worrisome example. In sage.rings.finite_rings.element_base we have that is_FiniteFieldElement(x) has a deprecation on it:

   deprecation(32664, "the function is_FiniteFieldElement is deprecated; use isinstance(x, sage.structure.element.FieldElement) and x.parent().is_finite() instead")

this is from https://github.com/sagemath/sage/pull/35119

a part of a push towards namespace packages to certainly ill-conceived modularization project.

Dima

 

Yet:

sage: isinstance(GF(2).0, sage.structure.element.FieldElement)
False

how is it possible that something got deprecated in favour of something that doesn't even work in the most basic example?

We do have that parent(GF(2).0).is_field() is true and that it inherits from sage.rings.ring.Field, so on the parent side things seem to be OK.

--
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+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/sage-devel/62332ced-2bdd-4ab4-9b80-934835a9697dn%40googlegroups.com.

[Nils Bruin]

On Monday, 28 April 2025 at 12:56:36 UTC-7 dim...@gmail.com wrote:

this is from https://github.com/sagemath/sage/pull/35119

a part of a push towards namespace packages to certainly ill-conceived modularization project.

 

I don't think the origin of the change is so relevant (other than the reputational damage people incur from having buggy code accepted in sagemath but that's more a cautionary tale for individual contributors to properly review their own work).  I think it's more of a wake-up call for our code quality review process. The deprecation remark suggests a statement that is not at all equivalent to the code that's there:

from sage.rings.finite_rings.finite_field_base import is_FiniteField

return isinstance(x, Element) and is_FiniteField(x.parent())

Most of these "is_*" routines used to be isinstance checks (as this one is mainly too). The tricky bit here is that it says "Element" and not "FieldElement". That's a big gotcha indeed. One would expect field elements to inherit from FieldElement and for finite fields that's not what happens. Wasn't the category framework supposed to fix such things?

[Vincent Delecroix]

Indeed, the suggestion has many corner cases.I think a most reasonable
version would be

parent(x) in Fields().Finite()

Vincent

> --
> 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+...@googlegroups.com.

> To view this discussion visit https://groups.google.com/d/msgid/sage-devel/92d4821f-c6f4-45cf-89ac-3863a8761cb5n%40googlegroups.com.