Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix docstring markup in sage/categories #34168

Closed
kwankyu opened this issue Jul 12, 2022 · 27 comments
Closed

Fix docstring markup in sage/categories #34168

kwankyu opened this issue Jul 12, 2022 · 27 comments

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 12, 2022

Part of #34157:

sage/categories/unique_factorization_domains.py:74:1: RST215 Inline interpreted text or phrase reference start-string without end-string.
sage/categories/unique_factorization_domains.py:223:1: RST303 Unknown directive type "seealso".
sage/categories/unique_factorization_domains.py:273:1: RST303 Unknown directive type "seealso".
sage/categories/fields.py:76:1: RST215 Inline interpreted text or phrase reference start-string without end-string.
sage/categories/category.py:1394:1: RST303 Unknown directive type "todo".
sage/categories/finite_coxeter_groups.py:204:1: RST303 Unknown directive type "todo".
sage/categories/finite_coxeter_groups.py:466:1: RST303 Unknown directive type "todo".
sage/categories/rings.py:388:1: RST215 Inline interpreted text or phrase reference start-string without end-string.
sage/categories/affine_weyl_groups.py:21:1: RST303 Unknown directive type "todo".
sage/categories/integral_domains.py:58:1: RST215 Inline interpreted text or phrase reference start-string without end-string.
sage/categories/facade_sets.py:189:1: RST205 Explicit markup ends without a blank line; unexpected unindent.
sage/categories/hecke_modules.py:111:1: RST215 Inline interpreted text or phrase reference start-string without end-string.
sage/categories/category_with_axiom.py:752:1: RST201 Block quote ends without a blank line; unexpected unindent.

CC: @mkoeppe @tscrim

Component: documentation

Author: John Palmieri

Branch/Commit: 80d7f8f

Reviewer: Matthias Koeppe

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

@kwankyu kwankyu added this to the sage-9.7 milestone Jul 12, 2022
@kwankyu

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:2

I don't understand this one:

sage/categories/category_with_axiom.py:752:1: RST201 Block quote ends without a blank line; unexpected unindent.

The relevant lines seem to be

Axioms defined upon other axioms
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

but I don't know what's wrong with that.

@jhpalmieri
Copy link
Member

Branch: u/jhpalmieri/categories-docfixes

@jhpalmieri
Copy link
Member

Commit: be5851f

@jhpalmieri
Copy link
Member

comment:4

So far this branch addresses the other issues, but not category_with_axiom.py.


New commits:

be5851ftrac 34168: fix docstring markup for src/categories

@fchapoton
Copy link
Contributor

comment:5

exact same issue for me, as I tried without seing that you did the job alreay

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 13, 2022

comment:7

Not sure about this change (and similar changes) on the branch:

diff --git a/src/sage/categories/hecke_modules.py b/src/sage/categories/hecke_modules.py
index 6590034..06dcb99 100644
--- a/src/sage/categories/hecke_modules.py
+++ b/src/sage/categories/hecke_modules.py
@@ -109,7 +109,7 @@ class HeckeModules(Category_module):
             INPUT:
 
             - ``Y`` -- an Hecke module
-            - ``category`` -- a subcategory of :class:`HeckeModules`() or None
+            - ``category`` -- a subcategory of :class:`HeckeModules` or None
 
             The sole purpose of this method is to construct the homset
             as a :class:`~sage.modular.hecke.homspace.HeckeModuleHomspace`. If

I think we would want to keep the parentheses there.
Does something like

+            - ``category`` -- a subcategory of :class:`HeckeModules` ``()`` or ``None``

work?

@jhpalmieri
Copy link
Member

comment:8

I'm not sure what you mean by "work". I believe that all of those changes are in underscore methods and so by default do not appear in the reference manual. If I use

- ``category`` -- a subcategory of :class:`HeckeModules` ``()`` or ``None``

then the output from M._Hom_? in the notebook looks ugly:

* "category" -- a subcategory of "HeckeModules" "()" or "None"

I can't successfully build the docs with underscore methods included, so it's hard to test more broadly.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2022

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

80d7f8ftrac 34168: more markup fixes for categories

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2022

Changed commit from be5851f to 80d7f8f

@jhpalmieri
Copy link
Member

comment:10

Okay, here is markup that I hope will make everyone happy.

@jhpalmieri
Copy link
Member

comment:11

Back to the category_with_axioms.py puzzle: if I make this change, the error goes away:

diff --git a/src/sage/categories/category_with_axiom.py b/src/sage/categories/category_with_axiom.py
index 237ad5dfed..302902a5e5 100644
--- a/src/sage/categories/category_with_axiom.py
+++ b/src/sage/categories/category_with_axiom.py
@@ -749,7 +749,7 @@ The infrastructure would then be in charge of building the appropriate
 arborescence under the hood. Or rely on some database (see discussion
 on :trac:`10963`, in particular at the end of comment 332).
 
-Axioms defined upon other axioms
+axioms defined upon other axioms
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Sometimes an axiom can only be defined when some other axiom

Can anyone explain this? A bug in flake8?

@jhpalmieri
Copy link
Member

comment:12

We could skip this file, but I would prefer if we could understand and solve the problem.

@jhpalmieri
Copy link
Member

comment:13

Maybe the issue is that "Axioms" is the title of the file: the file starts

r"""
Axioms

This documentation covers how to implement axioms and proceeds with an
...

If I change that to "Axiom" or "AXIOMS", the problem goes away. Maybe flake8 or its rst parser are (too?) aggressive in searching for duplicate headers? We could make this change, for example:

diff --git a/src/sage/categories/category_with_axiom.py b/src/sage/categories/category_with_axiom.py
index 237ad5dfed..665a303166 100644
--- a/src/sage/categories/category_with_axiom.py
+++ b/src/sage/categories/category_with_axiom.py
@@ -1,5 +1,5 @@
 r"""
-Axioms
+Axioms for categories
 
 This documentation covers how to implement axioms and proceeds with an
 overview of the implementation of the axiom infrastructure. It assumes

although I don't really like the idea of having to work around other people's bugs.

@jhpalmieri
Copy link
Member

comment:14

This bug seems to strike whenever

  • the title of the file is a single word, and
  • that word begins another line in the file

It doesn't seem to mind if the title word appears by itself on another line. If the title is "Axioms", then a line starting "Axioms are" will cause an error but a line containing just "Axioms" is fine.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 13, 2022

comment:15

A single-word first line seems to match the regex in
https://github.com/peterjc/flake8-rst-docstrings/blob/master/flake8_rst_docstrings.py#L197

@tscrim
Copy link
Collaborator

tscrim commented Jul 14, 2022

comment:16

Replying to @mkoeppe:

Not sure about this change (and similar changes) on the branch:

diff --git a/src/sage/categories/hecke_modules.py b/src/sage/categories/hecke_modules.py
index 6590034..06dcb99 100644
--- a/src/sage/categories/hecke_modules.py
+++ b/src/sage/categories/hecke_modules.py
@@ -109,7 +109,7 @@ class HeckeModules(Category_module):
             INPUT:
 
             - ``Y`` -- an Hecke module
-            - ``category`` -- a subcategory of :class:`HeckeModules`() or None
+            - ``category`` -- a subcategory of :class:`HeckeModules` or None
 
             The sole purpose of this method is to construct the homset
             as a :class:`~sage.modular.hecke.homspace.HeckeModuleHomspace`. If

I think we would want to keep the parentheses there.
Does something like

+            - ``category`` -- a subcategory of :class:`HeckeModules` ``()`` or ``None``

work?

I am fairly opposed to those parentheses; they are unnatural to me. I think we should simply remove them. If we decided we want them to be there, we should use the :class:`HeckeModules() ` syntax.

@jhpalmieri
Copy link
Member

comment:17

Replying to @mkoeppe:

A single-word first line seems to match the regex in
https://github.com/peterjc/flake8-rst-docstrings/blob/master/flake8_rst_docstrings.py#L197

So they should perhaps make the change docstring = docstring.replace(firstline, "") -> docstring = docstring.replace(firstline, "", 1) to just change the first occurrence.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

comment:18

Yes, that would be a simple fix. I have isolated a testcase:

r"""
Axioms

This documentation covers how to implement axioms and proceeds with an

Axioms defined upon other axioms
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Concretely, this is to be implemented by defining the new axiom in the

"""

gets transformed to:



This documentation covers how to implement axioms and proceeds with an

 defined upon other axioms
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Concretely, this is to be implemented by defining the new axiom in the
sage/categories/category_with_axiom.py:6:1: RST201 Block quote ends without a blank line; unexpected unindent.

which causes the error.

@jhpalmieri
Copy link
Member

comment:19

Yes, good catch, I think that's the problem. I've opened peterjc/flake8-rst-docstrings#65. With the proposed change, category_with_axioms.py doesn't show this error, and neither does misc/sagedoc.py (with the removal of "noreplace" causing similar problems).

@jhpalmieri
Copy link
Member

Author: John Palmieri

@jhpalmieri
Copy link
Member

comment:20

I'm leaving category_with_axiom.py as is.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

comment:21

Replying to @jhpalmieri:

Okay, here is markup that I hope will make everyone happy.

Thanks, looking good.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

comment:22

Replying to @tscrim:

I am fairly opposed to those parentheses; they are unnatural to me. I think we should simply remove them.

The parentheses, indicating that it's talking about the unique instance, not the class, seem to be used very widely in this context, as git grep 'subcategory of .*()' shows.

If we decided we want them to be there, we should use the :class:`HeckeModules() ` syntax.

That's what the current branch does.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

Reviewer: Matthias Koeppe

@jhpalmieri
Copy link
Member

comment:24

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 28, 2022

Changed branch from u/jhpalmieri/categories-docfixes to 80d7f8f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants