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/matroids, sage/geometry, sage/modules #34167

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

Fix docstring markup in sage/matroids, sage/geometry, sage/modules #34167

kwankyu opened this issue Jul 12, 2022 · 41 comments

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 12, 2022

Part of #34157:

sage/matroids/graphic_matroid.py:325:1: RST214 Inline literal start-string without end-string.
sage/modules/tutorial_free_modules.py:3:1: RST303 Unknown directive type "MODULEAUTHOR".
sage/modules/tensor_operations.py:250:1: RST303 Unknown directive type "todo".
sage/modules/tensor_operations.py:327:1: RST215 Inline interpreted text or phrase reference start-string without end-string.
sage/modules/free_module.py:455:1: RST303 Unknown directive type "todo".

Not fixed here (comment:3, comment:4)

sage/geometry/polyhedron/constructor.py:174:1: RST201 Block quote ends without a blank line; unexpected unindent.

Depends on #30448
Depends on #22349

CC: @tscrim @jhpalmieri

Component: documentation

Author: Frédéric Chapoton, John Palmieri

Branch: f9c8724

Reviewer: Kwankyu Lee

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

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

This comment has been minimized.

@fchapoton
Copy link
Contributor

Branch: u/chapoton/34167

@fchapoton
Copy link
Contributor

Commit: b37c1de

@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:2

in this one, I could not fix the issue in geometry..


New commits:

b37c1derst fixes in geometry matroids modules

@jhpalmieri
Copy link
Member

comment:3

This could be related to #34168 comment:13: the title of the file is "Polyhedra", and this somehow leads to it flagging a line starting with "Polyhedra" as being an error. If you change the first line of the file like this:

diff --git a/src/sage/geometry/polyhedron/constructor.py b/src/sage/geometry/polyhedron/constructor.py
index 4606d979cf..4b705b371a 100644
--- a/src/sage/geometry/polyhedron/constructor.py
+++ b/src/sage/geometry/polyhedron/constructor.py
@@ -1,5 +1,5 @@
 r"""
-Polyhedra
+Constructing polyhedra
 
 In this module, a polyhedron is a convex (possibly unbounded) set in
 Euclidean space cut out by a finite set of linear inequalities and

the error will go away. I think this is a bug in the lexer/parser/flake8/whatever, and I would rather someone fix that, but this is a workaround.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

comment:4

Let's defer this failure to a follow-up ticket

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

comment:5
diff --git a/src/sage/modules/tutorial_free_modules.py b/src/sage/modules/tutorial_free_modules.py
index 0808fe0..88810b2 100644
--- a/src/sage/modules/tutorial_free_modules.py
+++ b/src/sage/modules/tutorial_free_modules.py
@@ -1,7 +1,7 @@
 r"""
 Tutorial: Using free modules and vector spaces
 
-.. MODULEAUTHOR:: Jason Bandlow
+AUTHOR: Jason Bandlow
 
 In this tutorial, we show how to construct and manipulate free modules
 and vector spaces and their elements.

git grep MODULEAUTHOR shows that it is used in a number of other files and is also documented in the developer guide.

Should we remove it from the developer guide?

Or should we add it to the directives known to the linter?

@jhpalmieri
Copy link
Member

comment:7

I think we should:

  • add MODULEAUTHOR to the list of known directives, and
  • change show_authors to True so that the MODULEAUTHOR is actually displayed: its default is False, and it can be set on line 179 of sage_docbuild/conf.py:
diff --git a/src/sage_docbuild/conf.py b/src/sage_docbuild/conf.py
index c9b81a8ba7..5efcc9f86e 100644
--- a/src/sage_docbuild/conf.py
+++ b/src/sage_docbuild/conf.py
@@ -176,7 +176,7 @@ exclude_patterns = ['.build']
 
 # If true, sectionauthor and moduleauthor directives will be shown in the
 # output. They are ignored by default.
-#show_authors = False
+show_authors = True
 
 # Default lexer to use when highlighting code blocks, using the IPython
 # console lexers. 'ipycon' is the IPython console, which is what we want

@jhpalmieri
Copy link
Member

comment:8

(The directive is mainly used in the thematic tutorials where show_authors is specifically set to True, but it's used a few other places as well.)

@jhpalmieri
Copy link
Member

comment:9

The same passage in the Developer's guide mentions directives "image", "figure", and "autofunction". The last one is not used, but the others are. Do they need to be added?

@jhpalmieri
Copy link
Member

Changed branch from u/chapoton/34167 to u/jhpalmieri/34167

@jhpalmieri
Copy link
Member

Dependencies: 30448

@jhpalmieri
Copy link
Member

Changed commit from b37c1de to 3de846f

@jhpalmieri
Copy link
Member

Last 10 new commits:

dccceb4src/sage/parallel: Fix errors shown by tox -e rst
4cc5674src/sage/quadratic_forms: Fix errors shown by tox -e rst
a2f5ef6src/sage/repl: Fix some errors shown by tox -e rst
c2d0a91src/sage/rings: Fix some errors shown by tox -e rst
8fc8776src/sage/structure: Fix errors shown by tox -e rst
18121fesrc/sage/modules: Fix errors shown by tox -e rst
d66c66e.github/workflows/lint.yml: Do not fail when rst checks fail
0677319Fix a typo
918dfcdMerge branch 't/30448/public/30448' into t/34167/34167
3de846ftrac 34167: add MODULEAUTHOR to the list of known directives

@jhpalmieri
Copy link
Member

comment:12

(Adding to the list of directives requires adding #30448 as a dependency.)

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2022

comment:13

Replying to @jhpalmieri:

The same passage in the Developer's guide mentions directives "image", "figure",

I think these two may be built in to the rst spec, so no need to add

and "autofunction". The last one is not used, but the others are. Do they need to be added?

I guess autofunction would be good to add, for completeness of the sphinx directives

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

61c2fe8trac 34167: add MODULEAUTHOR to the list of known directives

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2022

Changed commit from 3de846f to 61c2fe8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ecd5e1atrac 34167: add MODULEAUTHOR, autofunction to the list of known directives

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2022

Changed commit from 61c2fe8 to ecd5e1a

@jhpalmieri
Copy link
Member

comment:16

Replying to @mkoeppe:

I guess autofunction would be good to add, for completeness of the sphinx directives

Done. Note that I did not revert the change from MODULEAUTHOR to AUTHOR for Jason Bandlow.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jul 15, 2022

comment:17

LGTM. Thank you.

@vbraun
Copy link
Member

vbraun commented Jul 28, 2022

comment:18

Merge failure on top of:

8d7c0022e2 Trac #34165: Fix docstring markup in sage/tests/book_stein_modform.py

07e1a716ad Trac #34162: Fix docstring markup in sage/doctest

3cfa5a1b95 Trac #34161: Fix docstring markup in sage/databases

411bb16ba0 Trac #34159: Fix docstring markup in sage/modular

1bc96cee2e Trac #34158: Fix docstring markup in sage/manifolds

87a05b5d35 Trac #34156: Fix docstring markup in sage/game_theory

af1bfa810c Trac #34153: pycodestyle cleanup in sage/geometry/hyperplane_arrangement/hyperplane.py

96ab5f3ffc Trac #34146: Modernize super in coding/

992333a3c7 Trac #34143: three-argument pow() on integers returns IntegerMod

84ea5a2927 Trac #34101: cutwidth_dyn use after free

7812ae01d9 Trac #34086: E231 in src/sage/graphs/strongly_regular_db.pyx - part 1

a9055ca510 Trac #34079: pycodestyle cleanup in 6 files of src/sage/graphs/

5ed3caeeb4 Trac #34078: pycodestyle cleanup in src/sage/graphs/graph_plot.py

785ba0960e Trac #34077: pycodestyle cleanup in src/sage/graphs/graph_input.py

691239d449 Trac #34073: pycodestyle cleanup in src/sage/graphs/graph_coloring.pyx

061241f24b Trac #34068: pycodestyle cleanup in src/sage/graphs/connectivity.pyx

65b834a993 Trac #33971: Height of a dynamical system is wrong

385b628ef7 Trac #22857: Using symbolic variables in domain 'positive' makes Sage crash

cc6a8fb370 Trac #10980: Make sure symbolic gridline values are okay

1a32d02790 Trac #34227: Compute the similarity class type of a given matrix; compute invariant subspace generating function from similarity class type

1f9fbb8fca Trac #34218: Implement longest_increasing_subsequence_number

d55f18bd80 Trac #34216: configure: Use --with-system-{gfortran,openssl,python3}=force on Cygwin

ffa1104f58 Trac #34214: Faster version of longest_increasing_subsequence_length

4c250eb283 Trac #34208: Remove package sip

e8caec9232 Trac #34200: Add importlib_metadata to Sphinx's dependencies

195b7578eb Trac #34189: Remove imports from sage.all and sage.rings.all in sage.rings

8117fe9e0f Trac #34187: Remove src/sage/init.py

99e0db84b0 Trac #34177: Update setuptools to 63.2.0

22e1027795 Trac #34176: update curl to version 7.84

68c736c3c9 Trac #33817: GH Actions: Add test of the modularized distributions

6b7bed7c35 Trac #32406: Make "./configure --enable-editable" the default

2c1e4a1a1b Trac #31568: Add commands "sage --lldb", "sage -t --lldb"

96b57ea0f5 Trac #31451: Faster version of longest_increasing_subsequences

991429de56 Trac #34115: tox.yml: Refactor using reusable workflows, update Linux platforms

05a53e3c45 Trac #33627: Remove sage-gdb-commands from src/bin

c17e6377ce Trac #22349: Deprecate sorting of Graph vertices and edges by default

c19c47f Trac #34174: insufficient precision in scaling elliptic curves over number fields by units

aa8a464 Trac #34172: Fix docstring markup in sage/groups and sage/misc

4142099 Trac #34168: Fix docstring markup in sage/categories

e9be172 Trac #34166: Fix docstring markup in sage/tests except sage/tests/book_stein_modform.py

6e41b53 Trac #34164: Fix docstring markup in sage/plot and sage/graphs

b03229e Trac #34163: Fix docstring markup in sage/interfaces

46849b8 Trac #34160: Fix docstring markup in sage/schemes

984137b Trac #34155: OpenSSL 3.0.5 security update

ced8f29 Trac #34151: get rid of Oops messages in quadratic forms

b045663 Trac #34149: make documentation building compatible with sphinx 5

07d4b90 Trac #34105: ZeroDivisionError while reducing a polynomial w.r.t. the zero ideal

db21903 Trac #33798: Implement the Links-Gould polynomial invariant for links

04ad7f9 Trac #33705: "make doc-clean" should remove inventory, doctrees

977e691 Trac #33636: replace loadable_module_extension() by importlib.machinery.EXTENSION_SUFFIXES

c744d7c Trac #29097: build/make/Makefile.in: Rename make targets SPKG-clean to SPKG-uninstall

8312ee1 Trac #33530: Upgrade ipython to 8.x

067a66c Trac #33428: prompt_toolkit 3.0.25+ breaks Ctrl-C

79ed9e5 Trac #33160: update Singular to 4.3.1

4cc4817 Trac #32088: gfan testsuite hangs on 32bit

10247d5 Trac #31049: "setup.py develop" rewrites the installed sage-version.sh as if it is a Python script

7f71494 Updated SageMath version to 9.7.beta6

merge was not clean: conflicts in src/sage/matroids/graphic_matroid.py

@jhpalmieri
Copy link
Member

comment:19

I would rather wait for the next beta to deal with this, rather than try to figure out where the merge conflict is.

@kwankyu
Copy link
Collaborator Author

kwankyu commented Jul 28, 2022

comment:20

Replying to @jhpalmieri:

I would rather wait for the next beta to deal with this, rather than try to figure out where the merge conflict is.

I agree.

On the other hand, I did try to find the conflicting ticket and found that it is #22349. It is tedious to do this manually. It would be useful if we have the command git trac merge 31049 32088 ... 34165 that stops at the ticket that has a merge conflict. Moreover the merge conflict message from the release manager could be so that we can just copy and paste the command. But deciding to actually put the found ticket as a dependency seems another matter though.

@jhpalmieri
Copy link
Member

comment:21

If that's the conflict, the patch is

diff --git a/src/sage/matroids/graphic_matroid.py b/src/sage/matroids/graphic_matroid.py
index f82a2145e9b..bd79b89e94f 100644
--- a/src/sage/matroids/graphic_matroid.py
+++ b/src/sage/matroids/graphic_matroid.py
@@ -344,7 +344,7 @@ class GraphicMatroid(Matroid):
              frozenset({4})]
         """
         star_list = []
-        for v in self._G.vertices():
+        for v in self._G.vertices(sort=False):
             star = [l for (_, _, l) in self._G.edges_incident(v)]
             star_list.append(frozenset(star))
         return frozenset(star_list)

@jhpalmieri
Copy link
Member

comment:22

(I'm actually surprised that #22349 hasn't caused more problems, since it's a bit of a patch bomb.)

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 1, 2022

Changed dependencies from #30448 to #30448, #22349

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 1, 2022

Changed branch from u/jhpalmieri/34167 to u/mkoeppe/34167

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 1, 2022

New commits:

ee62116Merge branch 'develop' of git://trac.sagemath.org/sage into t/34167/34167
1ec4bdftrac 22349: vertex sorting
a9ecbc5trac 22349: edge sorting
1e1ea24trac #22349: some corrections
4362caeMerge branch 'develop' into t/22349/public/graphs/22349_deprecate_sorting_vertices_and_edges
fe93716trac 22349: fix one more doctest
f9c8724Merge #22349

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 1, 2022

Changed commit from ecd5e1a to f9c8724

@kwankyu
Copy link
Collaborator Author

kwankyu commented Aug 1, 2022

comment:27

Okay.

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 4, 2022

comment:28

dependency of a "critical" ticket

@vbraun
Copy link
Member

vbraun commented Aug 6, 2022

Changed branch from u/mkoeppe/34167 to f9c8724

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 19, 2022

Changed commit from f9c8724 to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 19, 2022

Changed author from Frédéric Chapoton, ​John Palmieri to Frédéric Chapoton, John Palmieri

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

5 participants