Opened 4 years ago

# improve coding style and documentation in projective schemes — at Version 16

Reported by: Owned by: bhutz bhutz trivial sage-7.1 algebraic geometry Ben Hutz Frédéric Chapoton, Rebecca Lauren Miller, Joseph Eisner N/A u/bhutz/ticket/19891 (Commits) d8752ed6a585e944085833f2ac826cea3e757d08

Looks good to me.

### comment:1 Changed 4 years ago by bhutz

• Branch set to u/bhutz/ticket/19891

### comment:2 Changed 4 years ago by chapoton

• Commit set to 7aac718894d13b8f52373ec0f38d080dca3ed162
• Reviewers set to Frédéric Chapoton

Please put this ticket in "needs_review". I have assumed that you forgot to do so. Here are some points that I have seen at first reading.

```+    See [Hutz-gr]
```

is wrong. The correct refererence syntax is `[Hutz-gr]_` with an underscore at the end. This happen at many places, for example also for `See [FMV]` and `[Silverman_ADS]` and `[Bruin-Molnar] and [Molnar]`.

````PGL(2,QQ)`
```

could be

````PGL(2,\QQ)`
```

This looks strange:

```-        lift,lifted = blift(LG,Li,p,S=S)
+        lift,lifted = blift(LG,Li,p,S = S)
```

`Abelian variety` should keep its capital A

```+        returns ``True`` if this map is a morphism.
```

should be

```+        Return ``True`` if this map is a morphism.
```

`greens function` should be `Green's function`

`+ THis map must` should be `+ This map must`

typo `the subsgroup` at least twice

typo `post-critially`

typo `the the LLL`

duplicate reference `[Hutz_gr]` and `[Hutz-gr]` ?

typo `this projective spaces`

New commits:

 ​0db500b `19891: fixes for endPN_minimal_model.py` ​9fcac67 `19891: fixes for projective_homset.py` ​293dd6c `19891: fixes for endPN_automorphism_group.py` ​069751e `19891: fixes for projective_morphism.py` ​8c073bc `19891: fixes for projective_morphism_help.py and update references` ​d90476d `19891 fixes for projective_point.py` ​935c0ec `19891: fixes for projective_rational_point.py` ​ce4dac1 `19891: fixes for projective_space.py` ​7aac718 `19891: doc formatting clean-up`

### comment:3 Changed 4 years ago by git

• Commit changed from 7aac718894d13b8f52373ec0f38d080dca3ed162 to 24a7f72b936648d2fdc02941022bed78772d72d8

Branch pushed to git repo; I updated commit sha1. New commits:

 ​24a7f72 `19891: fix references and typos`

### comment:4 Changed 4 years ago by bhutz

• Authors set to Ben Hutz
• Status changed from new to needs_review

Thanks for taking a look at this, Frederic. Actually, I didn't put it as needs-review as I was going have my new students go over it this week to get used to the reviewing process since there are no significant/difficult changes here (and the references still weren't working correctly). Your review is much appreciated and any additional typos caught are important to fix. I've fixed the ones you found and I may as well set it to needs-review at this point.

That is good to know about the reference. I hadn't figured out yet why they were only linking for some of them, so you saved some time trying to determine what was happening.

### comment:5 Changed 4 years ago by rlmiller

• Reviewers changed from Frédéric Chapoton to Frédéric Chapoton, Rebecca Lauren Miller
• Status changed from needs_review to needs_work

Ticket 19891

no period for input or output consistency

file endPN_automorphism_group. html

• automorphism group QQ

Prime_lower_bound, formatting missing quotes automorism fixing pair

whole file

• some out put and input capitalized others not, some punch at end , others not
• capitalize output

file endPN_minimal_model.html

• func minimal model example spacing
• func affine minimal example spacing

file projective morphism.html

• func dyatomic polyomial references [mopa] need link

? why trac ticket references?

• func pgl minimal references need link
• func minimal model references need link, is self necessary?
• input check, quotes or not seen both ways
• func multiplier spectra output qqbar fix( check all too!)
• func normalize coordinates note formating, and gcd? base_ring?
• func nth iterate, to do formatinf and random question in example
• fun periodic points output self?
• func primes of bad reduction groebner capitalize
• check all output nones
• func wronskian_ideal capitalize jacobian
• func all_rational preimages remove self
• function automrophism group link reference [fmv]

file projective_point. html

• func dehomogenize punctuation in description
• func nth iterate kwds: Boolean, capitalize

### comment:6 Changed 4 years ago by eisnerj

• Reviewers changed from Frédéric Chapoton, Rebecca Lauren Miller to Frédéric Chapoton, Rebecca Lauren Miller, Joseph Eisner

Verify you meant to remove the dehomogenize step:

@@ -372,31 +372,30 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

Should these two errors match?:

@@ -2271,13 +2284,13 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

Check line breaks:

@ -413,8 +415,10 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -850,9 +857,11 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

Grammar:

@@ -1501,8 +1514,10 @@ class SchemeMorphism_point_projective_field(SchemeMorphism_point_projective_ring

@@ -280,15 +280,16 @@ def automorphism_group_QQ_fixedpoints(rational_function, return_functions = Fals

Remove reference to self:

@@ -2659,16 +2680,18 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

@@ -3567,11 +3596,11 @@ class SchemeMorphism_polynomial_projective_space_field(SchemeMorphism_polynomial

@@ -973,7 +989,7 @@ class ProjectiveSpace_field(ProjectiveSpace_ring):

@@ -4249,10 +4289,14 @@ class SchemeMorphism_polynomial_projective_space_finite_field(SchemeMorphism_pol

@@ -4291,14 +4335,15 @@ class SchemeMorphism_polynomial_projective_space_finite_field(SchemeMorphism_pol

@@ -2391,37 +2406,38 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

@@ -2360,8 +2374,9 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

Spacing Issues:

@@ -765,7 +764,7 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

@@ -803,16 +802,16 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

@@ -833,7 +832,7 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

@@ -4129,9 +4166,9 @@ class SchemeMorphism_polynomial_projective_space_finite_field(SchemeMorphism_pol

@@ -4159,7 +4199,7 @@ class SchemeMorphism_polynomial_projective_space_finite_field(SchemeMorphism_pol

@@ -45,9 +44,9 @@ def _fast_possible_periods(self,return_points=False):

@@ -102,59 +101,59 @@ def _fast_possible_periods(self,return_points=False):

@@ -182,58 +182,58 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -246,20 +246,20 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -293,58 +293,58 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -3010,12 +3039,14 @@ class SchemeMorphism_polynomial_projective_space(SchemeMorphism_polynomial):

@@ -565,53 +569,54 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -742,16 +749,16 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -759,8 +766,8 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -813,9 +820,9 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -882,20 +891,20 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -903,12 +912,12 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1064,8 +1075,8 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1075,8 +1086,8 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1085,8 +1096,8 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1176,14 +1187,14 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1228,26 +1239,26 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1262,30 +1273,31 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1359,7 +1372,7 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1368,10 +1381,10 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1395,8 +1408,8 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1406,7 +1419,7 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1414,8 +1427,8 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

@@ -1424,24 +1437,24 @@ class SchemeMorphism_point_projective_ring(SchemeMorphism_point):

### comment:7 Changed 4 years ago by git

• Commit changed from 24a7f72b936648d2fdc02941022bed78772d72d8 to ab8ad1cbd1b0020d5908ef98db6acf033db11dd1

Branch pushed to git repo; I updated commit sha1. New commits:

 ​ab8ad1c `19891: additional fixes from review.`

### comment:8 Changed 4 years ago by bhutz

• Status changed from needs_work to needs_review

### comment:9 Changed 4 years ago by rlmiller

• Status changed from needs_review to positive_review

### comment:10 Changed 4 years ago by vbraun

• Status changed from positive_review to needs_work

Merge conflict, try with the next beta

### comment:11 Changed 4 years ago by git

Branch pushed to git repo; I updated commit sha1. New commits:

 ​0fe9788 `Merge branch 'master' into ticket/19891`

### comment:12 Changed 4 years ago by bhutz

that still wasn't the right beta, I have it now and did another merge. Conflict resolved; am now testing.

### comment:13 Changed 4 years ago by git

Branch pushed to git repo; I updated commit sha1. New commits:

 ​ae6b53d `Merge branch 'master' into ticket/19891`

### comment:14 Changed 4 years ago by bhutz

• Status changed from needs_work to needs_review

conflict resolved

### comment:15 Changed 4 years ago by git

• Commit changed from ae6b53dad8e6c164bd0c491abd8c58e60865edc8 to d8752ed6a585e944085833f2ac826cea3e757d08

Branch pushed to git repo; I updated commit sha1. New commits:

 ​d8752ed `19891: reduce spacing in imports`

### comment:16 Changed 4 years ago by rlmiller

• Description modified (diff)
• Status changed from needs_review to positive_review
Note: See TracTickets for help on using tickets.