Opened 6 years ago
Last modified 2 months ago
#22173 needs_review enhancement
Shioda invariants for hyperelliptic curves with genus 3
Reported by:  Anna Somoza Henares  Owned by:  

Priority:  major  Milestone:  sage9.8 
Component:  number theory  Keywords:  days82, hyperelliptic curves, shioda invariants, days98 
Cc:  Christelle Vincent, Elisa Lorenzo García, Sorina, Marco Streng, David Kohel, Florian Bouyer, Jeroen Sijsling, Jesper Noordsij  Merged in:  
Authors:  Sorina Ionica, Elisa Lorenzo Garcia, Anna Somoza  Reviewers:  Vincent Delecroix, JeanPierre Flori 
Report Upstream:  N/A  Work issues:  
Branch:  public/22173 (Commits, GitHub, GitLab)  Commit:  ca3b22c40bd8af36dc978cc470dca0168aeea207 
Dependencies:  #26462, #27633  Stopgaps: 
Description
Sorina Ionica, Elisa Lorenzo Garcia and I implemented the computation of Shioda invariants for a given hyperelliptic curve of genus 3.
Following the implementation of the constructors of genus 2 curves over various fields we have added the corresponding classes for genus 3 hyperelliptic curves.
Change History (58)
comment:1 Changed 6 years ago by
Branch:  → u/annasomoza/22173shiodainv 

comment:2 Changed 6 years ago by
Commit:  → 2bc9d8e21236ebd2b5d3ce7aa3339597e71e55c9 

comment:3 Changed 6 years ago by
Commit:  2bc9d8e21236ebd2b5d3ce7aa3339597e71e55c9 → 153178f03f8cc32071891be8b9c05f46e9601f58 

Branch pushed to git repo; I updated commit sha1. New commits:
153178f  Fix extra empty lines

comment:4 Changed 6 years ago by
Cc:  Elisa Lorenzo García Sorina Marco Streng David Kohel Florian Bouyer added 

comment:5 Changed 6 years ago by
missing "of" in
Compute invariants genus 2 and 3 hyperelliptic curves via 'Ueberschiebung'
comment:6 Changed 6 years ago by
Cc:  Jeroen Sijsling added 

comment:7 Changed 6 years ago by
Status:  new → needs_review 

comment:8 Changed 6 years ago by
Commit:  153178f03f8cc32071891be8b9c05f46e9601f58 → 955cc7257947a8795575d42c575a05d259d662b1 

comment:9 Changed 6 years ago by
doc still does not build:
+[dochtml] [curves ] /home/u1/chapoton/sage/local/lib/python2.7/sitepackages/sage/schemes/hyperelliptic_curves/invariants.py:docstring of sage.schemes.hyperelliptic_curves.invariants.shioda_invariants:7: WARNING: Duplicate explicit target name: "sh". +[dochtml] [curves ] /home/u1/chapoton/sage/local/lib/python2.7/sitepackages/sage/schemes/hyperelliptic_curves/invariants.py:docstring of sage.schemes.hyperelliptic_curves.invariants.shioda_invariants:7: WARNING: duplicate citation Sh, other instance in /home/u1/chapoton/sage/src/doc/en/reference/curves/sage/schemes/hyperelliptic_curves/invariants.rst
comment:10 Changed 6 years ago by
Commit:  955cc7257947a8795575d42c575a05d259d662b1 → 1da5c7e9f1a6d3f95cf97289ff22fb9cdbdda4fe 

Branch pushed to git repo; I updated commit sha1. New commits:
1da5c7e  Add Shioda computation for h neq 0. Doc builds.

comment:11 Changed 6 years ago by
some remarks:
 remove the commented code if it is useless
 reference [Sh] should go the master reference file (see http://doc.sagemath.org/html/en/developer/coding_basics.html#sagesmasterbibliographyfile)
 compile the html doc and look at it to check that it looks correct
 you could use "lazy import" for your new modules.
comment:12 Changed 6 years ago by
Status:  needs_review → needs_work 

comment:13 Changed 6 years ago by
Cc:  Christelle Vincent added 

comment:14 Changed 5 years ago by
Commit:  1da5c7e9f1a6d3f95cf97289ff22fb9cdbdda4fe → f6c94f2abbcd7c757fdb9f7ee982c2052a435769 

Branch pushed to git repo; I updated commit sha1. New commits:
b5cec8d  work in progress

abf1b10  Add doctests

6f89277  Fix extra empty lines

f8bd519  Fix output type

d20d1a2  Fix minor doc error

e9bf8ef  Add Shioda computation for h neq 0. Doc builds.

bd25753  Added lazy import of shioda_invariants. Reference [Sh] moved to master reference file as [Sh1967]. Doc builds.

f6c94f2  Resolve commit

comment:15 Changed 5 years ago by
Milestone:  sage7.5 → sage8.1 

Status:  needs_work → needs_review 
comment:16 Changed 5 years ago by
Commit:  f6c94f2abbcd7c757fdb9f7ee982c2052a435769 → e1cee8756014d9afa769952d22cc3e78796ba426 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e1cee87  Added lazy import of shioda_invariants. Reference [Sh] moved to master reference file as [Sh1967]. Doc builds.

comment:17 Changed 5 years ago by
Now that you have moved the reference [Sh1967] to the master reference file, you should update the remaining link [Sh]_
and replace
.. [Sh] etc
by
 [Sh1967]_
in the REFERENCE block.
comment:18 Changed 5 years ago by
Commit:  e1cee8756014d9afa769952d22cc3e78796ba426 → dfed7a9a734fc3d37b2d8c4a7b453a6407ccefc3 

Branch pushed to git repo; I updated commit sha1. New commits:
dfed7a9  Remove remaining [Sh] reference.

comment:19 Changed 5 years ago by
Commit:  dfed7a9a734fc3d37b2d8c4a7b453a6407ccefc3 → a6a933180ad2e75f4d775520f4686f70a5f6713c 

Branch pushed to git repo; I updated commit sha1. New commits:
a6a9331  Change computation of shioda invariants of a curve (now consistent with magma). Fix copyright typos.

comment:20 Changed 5 years ago by
Milestone:  sage8.1 → sage8.0 

comment:21 Changed 5 years ago by
You are creating 4 new files and 4 new classes in order to support a single function which is 10 lines long! This is not worth the trouble. Curves of genus 0, 1, 2 are special enough to have dedicated classes. Do you think that genus 3 have that many special methods that it would make sense to have dedicated classes?
I would rather propose to have a method shioda_invariants
in the class HyperellipticCurve_generic
that does raise an error when the curve is not of genus 3.
comment:23 followup: 24 Changed 5 years ago by
I recall this ticket applying correctly before. It must have something to do with its age. I also recall that it returned the correct Shioda invariants. It is therefore a pity that it has not been incorporated in any form, since it is a worthwhile contribution.
The code is now there and certainly it can still be incorporated in some way. The question is how exactly, a question that is related to the general design of hyperelliptic curve classes. What complicates things is that before the authors' contributions can be incorporated, this needs to be decided on. Here are some thoughts; note I do not know if they are useful since I am not the biggest expert on Sage.
As for the new class creation, it strikes me as odd that what was all right in genus 2 should be problematic in genus 3. The authors had truly taken care to follow previous practice in the former genus to the letter. That is not a bad thing, rather the opposite in fact.
As for genus 3 curves being special or not, consider what there is in genus 2: a zoo of different invariants and the Kummer. That was enough for a new class. I agree with that, but then what is the content of "special enough"? In my opinion, genus 3 hyperelliptic curves are interesting enough to merit their own class if genus 2 curves are. Think of their Prym varieties and isogenies of small degree with plane quartics, for example.
The suggestion to have shioda_invariants
be a generic method of hyperelliptic curves means that igusa_clebsch_invariants
should be one too. It is not, so again the authors have followed established practice. A decision of this kind would also cause the number of invariants of a generic hyperelliptic curve to grow as new invariants in higher genus are discovered. Creating new classes in these genera as they start to receive more mathematical attention makes more sense to me.
Another alternative is to have shioda_invariants
be a global function, just like igusa_clebsch_invariants
. But is it justifiable to do this for every new set of invariants? In my opinion, it is better to expand the number of classes in small genus than to burden either the general hyperelliptic curve class or the global namespace with lots of functions.
Making methods and functions for a specific genus into general ones has been done before, not only for igusa_clebsch_invariants
but also for HyperellipticCurve_from_invariants
. This is another globally exported function, which assumes the invariants in question to be the IgusaClebsch invariants. But not all hyperelliptic curves have genus 2, and even then the IgusaClebsch invariants are not everyone's first choice. The same applies to the function Mestre_conic
, which also exists for more general hyperelliptic curves. I find this confusing. Perhaps these functions can later be generalized with some kind of keyword argument that gives the genus or the set of invariants in question, with appropriate defaults so as not to break reverse compatibility.
I am not sufficiently experienced to immediately find a good and consistent infrastructure here, but certainly the current situation makes it difficult to guess what to do with this code.
comment:24 followup: 25 Changed 5 years ago by
Thanks for making this ticket revives!
Replying to sijsling:
As for the new class creation, it strikes me as odd that what was all right in genus 2 should be problematic in genus 3. The authors had truly taken care to follow previous practice in the former genus to the letter. That is not a bad thing, rather the opposite in fact.
classes: there are 4 of them! The useful one HyperellipticCurve_g3_generic
and the trivial ones HyperellipticCurve_g3_finite_field
, HyperellipticCurve_g3_padic_field
, HyperellipticCurve_g3_rational_field
.
There should not be a class+file for each pair (genus, ring)
. If I am interested in hyperelliptic curves over QQbar[[x]]
I have to write classes for genus 2 and 3. And them, somebody likes genus 4 and starts writing 5 classes for generic
, finite_field
, padic_field
, rational_field
and qqbar_x
, etc. And each time, one class, one file. For me this is a terrible design. And you are somehow suggesting that it is better to follow a bad design decision rather than questionning the initial one!
As for genus 3 curves being special or not, consider what there is in genus 2: a zoo of different invariants and the Kummer. That was enough for a new class. I agree with that, but then what is the content of "special enough"? In my opinion, genus 3 hyperelliptic curves are interesting enough to merit their own class if genus 2 curves are. Think of their Prym varieties and isogenies of small degree with plane quartics, for example.
The suggestion to have
shioda_invariants
be a generic method of hyperelliptic curves means thatigusa_clebsch_invariants
should be one too. It is not, so again the authors have followed established practice. A decision of this kind would also cause the number of invariants of a generic hyperelliptic curve to grow as new invariants in higher genus are discovered. Creating new classes in these genera as they start to receive more mathematical attention makes more sense to me.Another alternative is to have
shioda_invariants
be a global function, just likeigusa_clebsch_invariants
. But is it justifiable to do this for every new set of invariants? In my opinion, it is better to expand the number of classes in small genus than to burden either the general hyperelliptic curve class or the global namespace with lots of functions.
In the current branch shioda_invariants
is first of all a function in sage/schemes/hyperelliptic_curves/invariants.py
. Of course this function should not be part of the global namespace.
Making methods and functions for a specific genus into general ones has been done before, not only for
igusa_clebsch_invariants
but also forHyperellipticCurve_from_invariants
. This is another globally exported function, which assumes the invariants in question to be the IgusaClebsch invariants. But not all hyperelliptic curves have genus 2, and even then the IgusaClebsch invariants are not everyone's first choice. The same applies to the functionMestre_conic
, which also exists for more general hyperelliptic curves. I find this confusing. Perhaps these functions can later be generalized with some kind of keyword argument that gives the genus or the set of invariants in question, with appropriate defaults so as not to break reverse compatibility.I am not sufficiently experienced to immediately find a good and consistent infrastructure here, but certainly the current situation makes it difficult to guess what to do with this code.
Indeed!
comment:25 Changed 5 years ago by
That is a quick reply, great!
Replying to vdelecroix:
There should not be a class+file for each pair
(genus, ring)
. If I am interested in hyperelliptic curves overQQbar[[x]]
I have to write classes for genus 2 and 3. And them, somebody likes genus 4 and starts writing 5 classes forgeneric
,finite_field
,padic_field
,rational_field
andqqbar_x
, etc. And each time, one class, one file. For me this is a terrible design. And you are somehow suggesting that it is better to follow a bad design decision rather than questionning the initial one!
Ah I see then. I did not in fact know that you were questioning the initial design in genus 2 since that was not completely clear to me from your previous comment. But stated like this your point is quite cogent. So do you mean that ideally also the genus 2 functionality should move to the generic class?
In the current branch
shioda_invariants
is first of all a function insage/schemes/hyperelliptic_curves/invariants.py
. Of course this function should not be part of the global namespace.
This also makes sense.
I am not sufficiently experienced to immediately find a good and consistent infrastructure here, but certainly the current situation makes it difficult to guess what to do with this code.
Indeed!
So leaving aside what happens with igusa_clebsch_invariants
and HyperellipticCurve_from_invariants
and its generalization, this seems to be the suggestion for the new code for Shioda invariants:
 At the least we create a function
shioda_invariants
in invariants.py that can later be found and used via a suitable import statement.  Optionally, we create a separate genus 3 class and make
shioda_invariants
into a method for that class. But part of your point is that this is not the way to go since then the number of classes used will tend to infinity with time.
Personally I agree in that if we can do with a generic class only then that is what should happen. There is probably no possibility to store the genus on construction and tabcomplete only those methods that apply for that genus? That would be optimal, but perhaps it is perverse and/or impossible to have the instance define the available methods.
comment:26 Changed 5 years ago by
Branch:  u/annasomoza/22173shiodainv → u/sijsling/22173shiodainv 

comment:27 Changed 5 years ago by
Branch:  u/sijsling/22173shiodainv → u/annasomoza/22173shiodainv 

comment:28 Changed 5 years ago by
Attempt to merge develop failed because trac cannot automerge the result. Should I rebase?
comment:29 Changed 5 years ago by
I figured out how to rebase this into a ticket for Sage 8.1. I can force push this so that the ticket applies. The result is that exactly the same changes are made, but that one needs to install the new version of Sage to use them (after installing trac as well and doing git trac checkout 22173 of course).
Shall I push this? I will not do so right away unless I hear an active yes from the authors and reader of this thread.
comment:30 Changed 5 years ago by
Here are some impressions of the global structure. This goes beyond the present ticket but I mention it anyway. Let X be a curve with Jacobian J. First some things that seem strange:
 The Kummer surface, which is in the global namespace, does not actually return anything and at present calls like
KummerSurface(J)
andJ.kummer_surface()
, andX.kummer_morphism
fail.
 All invariant functions are usual imports, but
shioda_invariants
is a lazy one. Why?
Here are some suggestions for change in various directions:
 I quite like the global export of
monsky_washnitzer
, which is not a function but a module, so essentially a bunch of functions bundled up. Would it be an idea to have similar modules calledcurve_invariants
andcurve_reconstruction
? This prevents the global namespace from being filled with the names of various alternative invariants, while still keeping these functions available.
 The question is of course how bad it is to export to the global namespace... this module is not the only one that does this. I would personally say that functions that are naturally methods should only be methods, and that functions that do not fit this mold, like
HyperellipticCurve_from_invariants
, should become global functions, whether in a container likecurvereconstruction
or not.
 As suggested by Vincent, we could remove the methods specific to a genus and have them be general ones. In the end I am also convinced that this does lead to better maintenance. The only problem is that there are lots of invariants. Here is a suggestion to avoid making these all into individual methods for general hyperelliptic curves. We could make a method
invariants
that depending on the curve chooses a particular set of invariants (perhaps IgusaClebsch in genus 2 and Shioda in genus 3, to be shown in the docstring). Giving a string to invariants allows the selection of other invariants, with an error message if no invariants corresponding to the string exist or if the genus does not match up. We could mention the various invariants in the docstring of the methodinvariants
, while writing more documentation in the moduleinvariants
(which could become the globalcurve_invariants
).
 The previous suggestion leads to the loss of tab completion. An alternative is to have an attribute
invariants
that is itself a class and that can be tabcompleted further to give the various individual invariant functions. Personally I think few people will care about tabcompleting invariants though, and if we at least mention the available invariants in the documentation ofinvariants
this should probably be OK.
The suggested changes do cause backward compatibility problems, mainly because of the functions that are currently available locally. If this cannot be changed, then at least one can generalize HyperellipticCurve_from_invariants
and Mestre_conic
later by having the current version as a default.
I will leave it at this for a while and wait for feedback :)
comment:31 Changed 5 years ago by
I do not think that rebasing to 8.1 helps either to get an automerge. Not only does this force everyone to upgrade to 8.1, but it would not even help. The automerge does not work because of a modification to src/doc/en/reference/references/index.rst
in commit d893f9
on the develop
branch... which is *after* the update to 8.1.
The additions of the authors are not really in conflict, but Git cannot automerge because it does not find a common ancestor. I have tried to split the comments in such a way that for one of them merging develop
is literally replacing an empty line with the new reference introduced in d893f9
, but that also fails.
comment:32 followups: 33 34 Changed 4 years ago by
I have not received any feedback to these suggestions, and by now the ticket is very old. In a nutshell, based on the preceding, I still suggest that two new tickets be created.
The first of these would make a method invariants
for general curves. For hyperelliptic curves of a genus 2 or 3, it would spit out a distinguished set of invariants. Other invariants, like the G2 invariants could be chosen by specifying a string. Later other invariants, like the DixmierOhno invariants, could be added.
The second would be the extension of HyperellipticCurve_from_invariants
and Mestre_conic
to higher genus, perhaps as methods in a more general module called reconstruction
, because these functions arguably clutter the global namespace. Alternatively, we could modify HyperellipticCurve
to accept a keyword argument specifying the invariants.
I will not create these tickets myself because I am not the main author of this code and do not want to tell its authors what to do, all the more since the preceding comments have merely been a long monologue on my part. But a third ticket, having to do with the behavior of the Kummer functionality, will now be filed by me as a bug report.
comment:33 Changed 4 years ago by
Replying to sijsling:
Replying to vdelecroix:
<SNIP>
Personally I agree in that if we can do with a generic class only then that is what should happen. There is probably no possibility to store the genus on construction and tabcomplete only those methods that apply for that genus? That would be optimal, but perhaps it is perverse and/or impossible to have the instance define the available methods.
In Python, you can create classes dynamically. That might be a solution.
Replying to sijsling:
The second would be the extension of
HyperellipticCurve_from_invariants
andMestre_conic
to higher genus, perhaps as methods in a more general module calledreconstruction
, because these functions arguably clutter the global namespace. Alternatively, we could modifyHyperellipticCurve
to accept a keyword argument specifying the invariants.
Another way that is sometimes used in Python/Sage? is to have explicit class functions for creating objects from a given datatype
sage: MyObject.from_equation(x^2 + 1) sage: MyObject.from_igusa_invariant("aa9b302")
That has the advantage of not complicating too much the __init__
constructor.
comment:34 followup: 35 Changed 4 years ago by
Cc:  Jesper Noordsij added 

Replying to sijsling:
I still suggest that two new tickets be created.
The first of these would make a method
invariants
for general curves. For hyperelliptic curves of a genus 2 or 3, it would spit out a distinguished set of invariants. Other invariants, like the G2 invariants could be chosen by specifying a string. Later other invariants, like the DixmierOhno invariants, could be added.
+1
The second would be the extension of
HyperellipticCurve_from_invariants
andMestre_conic
to higher genus, perhaps as methods in a more general module calledreconstruction
, because these functions arguably clutter the global namespace. Alternatively, we could modifyHyperellipticCurve
to accept a keyword argument specifying the invariants.
I agree. In detail:
 I'm sorry for putting
Mestre_conic
in the global namespace, it fits much better in a module.
 Letting
HyperellipticCurve
accept a keyword is better than having a separate function. I created the separate functionHyperellipticCurve_from_invariants
in order to make it consistent withEllipticCurve_from_j
, but since we may be getting moreandmore of these, I agree with letting go of the consistency.
 The name "
reconstruction
" might be a bit generic. However, there already is a module calledinvariant_theory
where this may fit.
Related to this discussion:
What really happens when constructing a hyperelliptic curve of genus g
(in characteristic different from 2) from the invariants is that a binary form of degree 2g+2
is reconstructed from its invariants. Reconstructing binary forms from invariants is useful also outside of hyperelliptic curves and also for odd degree. Degree 6 is currently in the HyperellipticCurve_from_invariants
function. Degree 5 is #25508. Degree 8 is #22173.
I think it would be good to have all the (binary and ternary) form reconstructions in the same place, and as this reconstruction for odd degree is unrelated to hyperelliptic curves, it should not be implemented inside the hyperelliptic curve functions. It fits much better in/around sage/rings/invariant_theory.py
. So I think the first step is to move the degree 6 code to the better location, and then implement the code for degree 5 and 8 there too. The code can then of course be wrapped by letting HyperellipticCurve
accept a keyword.
I'm ccing the author of #25508 who may have already started working on such a move.
comment:36 Changed 4 years ago by
Commit:  a6a933180ad2e75f4d775520f4686f70a5f6713c → 4a6a4809197181e5c268b95e64fdbd8e1bfa0c2f 

Branch pushed to git repo; I updated commit sha1. New commits:
daa89e4  Add dynamic creation of classes in constructor. Remove extra class files.

e4d5db4  Replace tabulations with spaces

1c35da7  Modify fieldrelated data treatment

2863afa  Doc now builds

e455b4d  Merge remotetracking branch 'origin/u/annasomoza/22173shiodainv' into localmerge22173

e51cb49  Changed to use dynamic class creation introduced in #27633

5b5c570  Modify genusrelated data treatment. Store alreadycreated classes for future use.

4a6a480  Merge branch 't/27633/redesign_of_the_constructor_for_hyperelliptic_curves' into localmerge22173

comment:37 Changed 4 years ago by
Dependencies:  #26462 → #26462, #27633 

Keywords:  days98 added 
Milestone:  sage8.0 → sage8.8 
Thank you all for your feedback. I finally found the time to deal with this! In #27633 there is a proposal for the issue with the design of classes, and I have updated the branch of this one to follow that method.
I agree that a general invariants
method would be good, but that should be a topic for another ticket.
comment:38 Changed 4 years ago by
Commit:  4a6a4809197181e5c268b95e64fdbd8e1bfa0c2f → eb73de805789c73c90d883f5909a89047ba2f6f3 

Branch pushed to git repo; I updated commit sha1. New commits:
9295319  Back to using a dictionary to deal with genusrelated classes

12069c7  Test added. Dynamic creation of classes is now done with `dynamic_class` function.

fe002eb  Add missing endline in documentation

90459d4  Merge branch 't/27633/redesign_of_the_constructor_for_hyperelliptic_curves' into localmerge22173

eb73de8  Add inheritance test to doc.

comment:39 Changed 4 years ago by
Status:  needs_work → needs_review 

#27633 has now positive review, what I think solves all main issues raised here.
comment:40 Changed 4 years ago by
Reviewers:  → Vincent Delecroix 

Status:  needs_review → positive_review 
comment:41 Changed 4 years ago by
Commit:  eb73de805789c73c90d883f5909a89047ba2f6f3 → dcff833a67c84b73a34b088585eae9f14474a83f 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
dcff833  Remove unused variable

comment:42 followup: 44 Changed 4 years ago by
Status:  needs_review → needs_work 

and the branch is now red, meaning some conflicts with the latest develop version of sage
By the way, never ever make any change to some ticket when it is in positive_review..
comment:43 Changed 3 years ago by
Commit:  dcff833a67c84b73a34b088585eae9f14474a83f → f0d91dee06f67dd2a131deb5241bc3715436b11f 

Branch pushed to git repo; I updated commit sha1. New commits:
f0d91de  Merge with latest beta

comment:44 Changed 3 years ago by
Status:  needs_work → needs_review 

Now it merges correctly. And sorry about changing it after the positive review, I thought that I had to correct the pyflake error.
comment:45 Changed 3 years ago by
Can someone review? Only change made after last positive_review is removing an unused variable.
comment:46 Changed 3 years ago by
Reviewers:  Vincent Delecroix → Vincent Delecroix, JeanPierre Flori 

Status:  needs_review → positive_review 
Looks ok.
comment:47 Changed 3 years ago by
Milestone:  sage8.8 → sagepending 

comment:49 Changed 2 years ago by
Milestone:  sagepending → sage9.2 

Status:  positive_review → needs_work 
Please rebase.
comment:50 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:51 Changed 22 months ago by
Milestone:  sage9.3 → sage9.4 

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:52 Changed 17 months ago by
Milestone:  sage9.4 → sage9.5 

Setting a new milestone for this ticket based on a cursory review.
comment:53 Changed 14 months ago by
Branch:  u/annasomoza/22173shiodainv → public/22173 

Commit:  f0d91dee06f67dd2a131deb5241bc3715436b11f → d989bb7d3404e0032b53f1b40929e627b6496f0c 
Status:  needs_work → needs_review 
I took the liberty to rebase this; no complications. Needs another review, and presumably also an OK from the authors.
Regarding #26462: This looks like a reminder to refactor things at some point rather than an actual dependency?
Last 10 new commits:
dfed7a9  Remove remaining [Sh] reference.

a6a9331  Change computation of shioda invariants of a curve (now consistent with magma). Fix copyright typos.

e455b4d  Merge remotetracking branch 'origin/u/annasomoza/22173shiodainv' into localmerge22173

e51cb49  Changed to use dynamic class creation introduced in #27633

4a6a480  Merge branch 't/27633/redesign_of_the_constructor_for_hyperelliptic_curves' into localmerge22173

90459d4  Merge branch 't/27633/redesign_of_the_constructor_for_hyperelliptic_curves' into localmerge22173

eb73de8  Add inheritance test to doc.

dcff833  Remove unused variable

f0d91de  Merge with latest beta

d989bb7  Merge tag '9.5.beta2' into u/annasomoza/22173shiodainv

comment:55 Changed 14 months ago by
Commit:  d989bb7d3404e0032b53f1b40929e627b6496f0c → ca3b22c40bd8af36dc978cc470dca0168aeea207 

Branch pushed to git repo; I updated commit sha1. New commits:
ca3b22c  remove __future__ import

comment:56 Changed 11 months ago by
Milestone:  sage9.5 → sage9.6 

Stalled in needs_review
or needs_info
; likely won't make it into Sage 9.5.
comment:57 Changed 8 months ago by
Milestone:  sage9.6 → sage9.7 

comment:58 Changed 2 months ago by
Milestone:  sage9.7 → sage9.8 

Branch pushed to git repo; I updated commit sha1. New commits:
work in progress
Merge branch 'develop' into localg3invariants
Add doctests