Skip to content

Conversation

JohnCremona
Copy link
Member

@JohnCremona JohnCremona commented Aug 10, 2023

This corrects a typo in a docstring concerning subgroups of the modular group. The typo made a definition mathematically incorrect.

Fixes #36058

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have updated the documentation accordingly.

⌛ Dependencies

No dependencies: based on 10.1.beta9

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@JohnCremona
Copy link
Member Author

When I look at the documentation preview at the link here, I see the old wrong docstring and not the corrected one. That does not seem right to me

@JohnCremona
Copy link
Member Author

https://deploy-preview-36062--sagemath-tobias.netlify.app/reference/arithgroup/sage/modular/arithgroup/congroup_gammah#sage.modular.arithgroup.congroup_gammaH.GammaH_constructor looks fine to me

Yes, that is OK -- but not the similar docstring at the top of that page! I found that place in the source file and will update the PR.

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it was in 2 places in the file. LGTM.

@github-actions
Copy link

Documentation preview for this PR (built with commit 1b1f336; changes) is ready! 🎉

vbraun pushed a commit to vbraun/sage that referenced this pull request Aug 11, 2023
This corrects a typo in a docstring concerning subgroups of the modular
group.  The typo made a definition mathematically incorrect.  See issue
sagemath#36058

- [x ] The title is concise, informative, and self-explanatory.
- [ x] The description explains in detail what this PR is about.
- [ x] I have linked a relevant issue or discussion.

- [ x] I have updated the documentation accordingly.

### ⌛ Dependencies

No dependencies: based on 10.1.beta9

URL: sagemath#36062
Reported by: John Cremona
Reviewer(s): David Coudert
@vbraun vbraun merged commit 517cdce into sagemath:develop Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typo in documentation for congruence subgroups
4 participants