sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

Fix typos and review the documentation of modform_hecketriangle #31898

Open DavidAyotte opened 3 years ago

DavidAyotte commented 3 years ago

The documentation of the package modular/modform_hecketriangle contains some typos in the documentation and inconsistencies regarding the official SageMath conventions. Here is the documentation of this package:

https://doc.sagemath.org/html/en/reference/modfrm_hecketriangle/index.html

Moreover, it would be desirable to add some reference and explain how to use the package more deeply. Indeed, the code here seems independent of the original modular forms implementation but use a different approach for its implementation. Thus, it can be confusing to use for someone who is already familiar to the original implementation of modular forms (by "original implementation" I mean the one in modular/modform).

Therefore, the goal of this ticket is to review the documentation, fix the typos and add references.

Remark: the package modform_hecketriangle was originally implemented in 2013 by Jonas Jermann (see the ticket #16134).

CC: @videlec

Component: modular forms

Keywords: modular forms hecke triangle gsoc2021 documentation style

Author: David Ayotte

Branch/Commit: u/gh-DavidAyotte/fix_typos_and_review_the_documentation_of_modform_hecketriangle @ 1b4a8e3

Issue created by migration from https://trac.sagemath.org/ticket/31898

DavidAyotte commented 3 years ago

Branch: u/gh-DavidAyotte/fix_typos_and_review_the_documentation_of_modform_hecketriangle

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

e399bd1Merge branch 'develop' into t/31898/fix_typos_and_review_the_documentation_of_modform_hecketriangle
e62b753typos and formatting, add a reference in readme.py
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Commit: e62b753

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

83cc96ffix NOTE and WARNING blocks, apply PEP8 to keywords, latex formatting
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from e62b753 to 83cc96f

fchapoton commented 3 years ago
comment:6

nneds review ?

DavidAyotte commented 3 years ago
comment:7

I haven't finished yet!

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 83cc96f to 697f0b8

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

3c8580clatex, PEP8, small fix
697f0b8titles capitalization
videlec commented 3 years ago
comment:10

Hi David,

What is the reason for capitalization here

-Graded rings of modular forms for Hecke triangle groups
+Graded Rings of Modular Forms for Hecke Triangle Groups
---------------------------------------------------------
-Modular forms for Hecke triangle groups
+Modular Forms for Hecke Triangle Groups
---------------------------------------------------------
-Analytic types of modular forms.
+Analytic Types of Modular Forms.
---------------------------------------------------------
etc
DavidAyotte commented 3 years ago
comment:11

I did that because most of the nouns in a title in english should be capitalized. Also, if you look at the reference manual, most of the titles follows this convention. The titles were not properly capitalized in this package, see:

https://doc.sagemath.org/html/en/reference/modfrm_hecketriangle/index.html

DavidAyotte commented 3 years ago

Author: David Ayotte

DavidAyotte commented 3 years ago
comment:13

Ok, I've look at the reference manual more closely and it seems that there is some inconsistencies about the capitalization of titles. See for example:

https://doc.sagemath.org/html/en/reference/rings_standard/index.html

You can find there two different styles (non-capitalized titles and capitalized titles)...

EDIT: I don't know what is the correct convention in SageMath, but if people prefer non-capitalized titles, I can revert back the commit. But I believe that the documentation should be consistent (with whatever convention is chosen)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 697f0b8 to 1b4a8e3

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

1b4a8e3Revert "titles capitalization"
DavidAyotte commented 3 years ago
comment:15

Replying to @videlec:

What is the reason for capitalization here

After some investigation, it turns out that the Python's guidelines for documentation is that section should not be in title case so commit 697f0b8 was a mistake from my part.

Source: https://devguide.python.org/documenting/?highlight=title%20case#capitalization

mkoeppe commented 2 years ago
comment:16

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

fchapoton commented 2 years ago
comment:18

red branch => needs work