From be07520c1a8c7499c0a9ad3c651a9aae00fb2629 Mon Sep 17 00:00:00 2001 From: Giacomo Pope Date: Thu, 14 Mar 2024 14:21:31 +0000 Subject: [PATCH 1/5] change construction and add doctest --- src/sage/schemes/hyperelliptic_curves/constructor.py | 10 ++++++++-- .../schemes/hyperelliptic_curves/hyperelliptic_g2.py | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/sage/schemes/hyperelliptic_curves/constructor.py b/src/sage/schemes/hyperelliptic_curves/constructor.py index faba7145ba5..4cc9abd1b91 100644 --- a/src/sage/schemes/hyperelliptic_curves/constructor.py +++ b/src/sage/schemes/hyperelliptic_curves/constructor.py @@ -277,7 +277,13 @@ def is_pAdicField(x): cls_name.append(name) break + base_cls = None + if len(superclass) == 0: + base_cls = HyperellipticCurve_generic + class_name = "_".join(cls_name) - cls = dynamic_class(class_name, tuple(superclass), - HyperellipticCurve_generic, doccls=HyperellipticCurve) + cls = dynamic_class(class_name, + tuple(superclass), + cls=base_cls, + doccls=HyperellipticCurve) return cls(PP, f, h, names=names, genus=g) diff --git a/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py b/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py index 7a8fe986448..5e5225ebff1 100644 --- a/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py +++ b/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py @@ -45,6 +45,15 @@ def jacobian(self): sage: f = x^5 - x^4 + 3 sage: HyperellipticCurve(f).jacobian() Jacobian of Hyperelliptic Curve over Rational Field defined by y^2 = x^5 - x^4 + 3 + + TESTS: + + Ensure that :issue:`37612` is fixed:: + + sage: R. = QQ[] + sage: f = x^5 - x^4 + 3 + sage: HyperellipticCurve(f).jacobian + """ return jacobian_g2.HyperellipticJacobian_g2(self) From 8adab0f1f2aad0ee3edf8531c203ab89f4a937f3 Mon Sep 17 00:00:00 2001 From: Giacomo Pope Date: Thu, 14 Mar 2024 16:55:28 +0000 Subject: [PATCH 2/5] change how construction is done --- .../hyperelliptic_curves/constructor.py | 87 +++++++++++-------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/src/sage/schemes/hyperelliptic_curves/constructor.py b/src/sage/schemes/hyperelliptic_curves/constructor.py index 4cc9abd1b91..f5507dac5aa 100644 --- a/src/sage/schemes/hyperelliptic_curves/constructor.py +++ b/src/sage/schemes/hyperelliptic_curves/constructor.py @@ -92,11 +92,11 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): sage: HyperellipticCurve(x^8 + 1, x) Traceback (most recent call last): ... - ValueError: Not a hyperelliptic curve: highly singular at infinity. + ValueError: not a hyperelliptic curve: highly singular at infinity sage: HyperellipticCurve(x^8 + x^7 + 1, x^4) Traceback (most recent call last): ... - ValueError: Not a hyperelliptic curve: singularity in the provided affine patch. + ValueError: not a hyperelliptic curve: singularity in the provided affine patch sage: F. = PowerSeriesRing(FiniteField(2)) sage: P. = PolynomialRing(FractionField(F)) @@ -128,7 +128,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): sage: HyperellipticCurve((x^3-x+2)^2*(x^6-1)) Traceback (most recent call last): ... - ValueError: Not a hyperelliptic curve: singularity in the provided affine patch. + ValueError: not a hyperelliptic curve: singularity in the provided affine patch sage: HyperellipticCurve((x^3-x+2)^2*(x^6-1), check_squarefree=False) Hyperelliptic Curve over Finite Field of size 7 defined by @@ -147,7 +147,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): sage: HyperellipticCurve(f, h) Traceback (most recent call last): ... - ValueError: Not a hyperelliptic curve: highly singular at infinity. + ValueError: not a hyperelliptic curve: highly singular at infinity sage: HyperellipticCurve(F) Hyperelliptic Curve over Rational Field defined by y^2 = x^6 + 1 @@ -160,7 +160,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): sage: HyperellipticCurve(x^5 + t) Traceback (most recent call last): ... - ValueError: Not a hyperelliptic curve: singularity in the provided affine patch. + ValueError: not a hyperelliptic curve: singularity in the provided affine patch Input with integer coefficients creates objects with the integers as base ring, but only checks smoothness over `\QQ`, not over Spec(`\ZZ`). @@ -203,18 +203,18 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): """ # F is the discriminant; use this for the type check # rather than f and h, one of which might be constant. - F = h**2 + 4*f + F = h**2 + 4 * f if not isinstance(F, Polynomial): - raise TypeError("Arguments f (= %s) and h (= %s) must be polynomials" % (f, h)) + raise TypeError(f"Arguments {f = } and {h = } must be polynomials") P = F.parent() f = P(f) h = P(h) df = f.degree() - dh_2 = 2*h.degree() + dh_2 = 2 * h.degree() if dh_2 < df: - g = (df-1)//2 + g = (df - 1) // 2 else: - g = (dh_2-1)//2 + g = (dh_2 - 1) // 2 if check_squarefree: # Assuming we are working over a field, this checks that after # resolving the singularity at infinity, we get a smooth double cover @@ -222,16 +222,20 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): if P(2) == 0: # characteristic 2 if h == 0: - raise ValueError("In characteristic 2, argument h (= %s) must be non-zero." % h) - if h[g+1] == 0 and f[2*g+1]**2 == f[2*g+2]*h[g]**2: - raise ValueError("Not a hyperelliptic curve: " - "highly singular at infinity.") - should_be_coprime = [h, f*h.derivative()**2+f.derivative()**2] + raise ValueError( + f"for characteristic 2, argument {h = } must be non-zero" + ) + if h[g + 1] == 0 and f[2 * g + 1] ** 2 == f[2 * g + 2] * h[g] ** 2: + raise ValueError( + "not a hyperelliptic curve: highly singular at infinity" + ) + should_be_coprime = [h, f * h.derivative() ** 2 + f.derivative() ** 2] else: # characteristic not 2 - if F.degree() not in [2*g+1, 2*g+2]: - raise ValueError("Not a hyperelliptic curve: " - "highly singular at infinity.") + if F.degree() not in [2 * g + 1, 2 * g + 2]: + raise ValueError( + "not a hyperelliptic curve: highly singular at infinity" + ) should_be_coprime = [F, F.derivative()] try: smooth = should_be_coprime[0].gcd(should_be_coprime[1]).degree() == 0 @@ -239,23 +243,33 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): try: smooth = should_be_coprime[0].resultant(should_be_coprime[1]) != 0 except (AttributeError, NotImplementedError, TypeError): - raise NotImplementedError("Cannot determine whether " - "polynomials %s have a common root. Use " - "check_squarefree=False to skip this check." % - should_be_coprime) + raise NotImplementedError( + "cannot determine whether " + f"polynomials {should_be_coprime} have a common root, use " + "check_squarefree=False to skip this check" + ) if not smooth: - raise ValueError("Not a hyperelliptic curve: " - "singularity in the provided affine patch.") + raise ValueError( + "not a hyperelliptic curve: " + "singularity in the provided affine patch" + ) R = P.base_ring() PP = ProjectiveSpace(2, R) if names is None: names = ["x", "y"] - superclass = [] + bases = [] cls_name = ["HyperellipticCurve"] + # For certain genus we specialise to child classes with + # optimised methods genus_classes = {2: HyperellipticCurve_g2} + if g in genus_classes: + bases.append(genus_classes[g]) + cls_name.append("g%s" % g) + # For certain base fields, we specialise to child classes + # with special case methods def is_FiniteField(x): return isinstance(x, FiniteField) @@ -265,25 +279,22 @@ def is_pAdicField(x): fields = [ ("FiniteField", is_FiniteField, HyperellipticCurve_finite_field), ("RationalField", is_RationalField, HyperellipticCurve_rational_field), - ("pAdicField", is_pAdicField, HyperellipticCurve_padic_field)] - - if g in genus_classes: - superclass.append(genus_classes[g]) - cls_name.append("g%s" % g) + ("pAdicField", is_pAdicField, HyperellipticCurve_padic_field), + ] for name, test, cls in fields: if test(R): - superclass.append(cls) + bases.append(cls) cls_name.append(name) break - base_cls = None - if len(superclass) == 0: - base_cls = HyperellipticCurve_generic + # If no specialised child class was found, we simply use the + # generic class in the class construction + if not bases: + bases = [HyperellipticCurve_generic] + # Dynamically build a class from multiple inheritance. Note that + # all classes we slect from are children of HyperellipticCurve_generic class_name = "_".join(cls_name) - cls = dynamic_class(class_name, - tuple(superclass), - cls=base_cls, - doccls=HyperellipticCurve) + cls = dynamic_class(class_name, tuple(bases), doccls=HyperellipticCurve) return cls(PP, f, h, names=names, genus=g) From 97620cc928278712ed768b2c44badcef2ed00964 Mon Sep 17 00:00:00 2001 From: Giacomo Pope Date: Thu, 14 Mar 2024 16:57:17 +0000 Subject: [PATCH 3/5] missed a captial --- src/sage/schemes/hyperelliptic_curves/constructor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/schemes/hyperelliptic_curves/constructor.py b/src/sage/schemes/hyperelliptic_curves/constructor.py index f5507dac5aa..65970b54725 100644 --- a/src/sage/schemes/hyperelliptic_curves/constructor.py +++ b/src/sage/schemes/hyperelliptic_curves/constructor.py @@ -205,7 +205,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): # rather than f and h, one of which might be constant. F = h**2 + 4 * f if not isinstance(F, Polynomial): - raise TypeError(f"Arguments {f = } and {h = } must be polynomials") + raise TypeError(f"arguments {f = } and {h = } must be polynomials") P = F.parent() f = P(f) h = P(h) From 35a1903a350d3bea2b59736b3bf295d5a3425be3 Mon Sep 17 00:00:00 2001 From: Giacomo Pope Date: Mon, 18 Mar 2024 23:43:24 +0000 Subject: [PATCH 4/5] reviewer changes --- src/sage/schemes/hyperelliptic_curves/constructor.py | 7 +++---- src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/sage/schemes/hyperelliptic_curves/constructor.py b/src/sage/schemes/hyperelliptic_curves/constructor.py index 65970b54725..d647312ee30 100644 --- a/src/sage/schemes/hyperelliptic_curves/constructor.py +++ b/src/sage/schemes/hyperelliptic_curves/constructor.py @@ -250,8 +250,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): ) if not smooth: raise ValueError( - "not a hyperelliptic curve: " - "singularity in the provided affine patch" + "not a hyperelliptic curve: singularity in the provided affine patch" ) R = P.base_ring() PP = ProjectiveSpace(2, R) @@ -266,9 +265,9 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): genus_classes = {2: HyperellipticCurve_g2} if g in genus_classes: bases.append(genus_classes[g]) - cls_name.append("g%s" % g) + cls_name.append(f"g{g}") - # For certain base fields, we specialise to child classes + # For certain base fields, we specialise to subclasses # with special case methods def is_FiniteField(x): return isinstance(x, FiniteField) diff --git a/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py b/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py index 5e5225ebff1..23cbaa7c9f1 100644 --- a/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py +++ b/src/sage/schemes/hyperelliptic_curves/hyperelliptic_g2.py @@ -52,8 +52,8 @@ def jacobian(self): sage: R. = QQ[] sage: f = x^5 - x^4 + 3 - sage: HyperellipticCurve(f).jacobian - + sage: type(HyperellipticCurve(f).jacobian()) + """ return jacobian_g2.HyperellipticJacobian_g2(self) From 2ce508e7e0a72ed894a682bca853e3162e652250 Mon Sep 17 00:00:00 2001 From: Giacomo Pope Date: Tue, 19 Mar 2024 07:12:27 +0000 Subject: [PATCH 5/5] reviewer changes --- src/sage/schemes/hyperelliptic_curves/constructor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sage/schemes/hyperelliptic_curves/constructor.py b/src/sage/schemes/hyperelliptic_curves/constructor.py index d647312ee30..8ed20d9ce5a 100644 --- a/src/sage/schemes/hyperelliptic_curves/constructor.py +++ b/src/sage/schemes/hyperelliptic_curves/constructor.py @@ -260,7 +260,7 @@ def HyperellipticCurve(f, h=0, names=None, PP=None, check_squarefree=True): bases = [] cls_name = ["HyperellipticCurve"] - # For certain genus we specialise to child classes with + # For certain genus we specialise to subclasses with # optimised methods genus_classes = {2: HyperellipticCurve_g2} if g in genus_classes: @@ -287,13 +287,13 @@ def is_pAdicField(x): cls_name.append(name) break - # If no specialised child class was found, we simply use the + # If no specialised subclasses are identified, we simply use the # generic class in the class construction if not bases: bases = [HyperellipticCurve_generic] # Dynamically build a class from multiple inheritance. Note that - # all classes we slect from are children of HyperellipticCurve_generic + # all classes we select from are subclasses of HyperellipticCurve_generic class_name = "_".join(cls_name) cls = dynamic_class(class_name, tuple(bases), doccls=HyperellipticCurve) return cls(PP, f, h, names=names, genus=g)