Fixed the doc `sets/recursively_enumerated_set.py` #37331

janmenjayap · 2024-02-14T07:18:19Z

Modified the doc in sets/recursively_enumerated_set.py (in the section Example: Forest structure) to display the HTML nicely; also corrected some typos.

Sphinx .. MATH:: directive is unable to render the picture environment in LaTeX, as a result of which the image is not begin displayed nicely in sets/recursively_enumerated_set. I generated the pdf and png of the required image by compiling the LaTeX code and attached the image. The inspiration is from Figure: The five complete binary trees with four leaves in combinat/tutorial.

Fixes #37084.

The link of the original documentation -- sets/recursively_enumerated_set.

📝 Checklist

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 created tests covering the changes. (Not required it seems.)
I have updated the documentation accordingly.

⌛ Dependencies

None.

grhkm21 · 2024-02-14T07:33:05Z

I'm not sure creating a new media is a good idea, since it's hard to change. Instead, try following how combinat/bijectionist does it. You can see the result here

janmenjayap · 2024-02-14T07:45:36Z

Hi,
I tried recreating the same stuff using arrays in a local latex environment as follows and it worked.

% LaTeX
\begin{equation*}
\begin{array}{rcl}
    % Root and Legend
    & \text{``\ "} \\
    \text{2) A function of an element enumerating}\quad \quad& \scalebox{1.5}{$\bullet$} & \scalebox{2}{$\leftarrow$}\quad\text{1) An initial element} \\
    \text{its children (if any) \hspace{3.75cm}} & & \\

    % Arrows
    \scalebox{2}{$\searrow$}\hspace{2cm} \scalebox{2}{$\swarrow$}  & \scalebox{2}{$\downarrow$} & \scalebox{2}{$\searrow$} \\[5pt]

    % First Children
    \text{``a"} \hspace{4.5em}& \text{``b"} & \hspace{4.5em}\text{``c"} \\
    \scalebox{1.5}{$\bullet$}\hspace{5em} & \scalebox{1.5}{$\bullet$} & \hspace{5em}\scalebox{1.5}{$\bullet$}\\

    % Second Children
    \begin{array}{rcl}
        \scalebox{2}{$\swarrow$}  & \scalebox{2}{$\downarrow$} & \scalebox{2}{$\searrow$} \\[5pt]
        \text{``aa"} \quad& \text{``ab"} & \quad\text{``ac"} \\
        \scalebox{1.5}{$\bullet$}\hspace{1.65em} & \scalebox{1.5}{$\bullet$} & \hspace{1.65em}\scalebox{1.5}{$\bullet$}\\
    \end{array} & 
    \begin{array}{rcl}
        \scalebox{2}{$\swarrow$}  & \scalebox{2}{$\downarrow$} & \scalebox{2}{$\searrow$} \\[5pt]
        \text{``ba"} \quad& \text{``bb"} & \quad\text{``bc"} \\
        \scalebox{1.5}{$\bullet$}\hspace{1.65em} & \scalebox{1.5}{$\bullet$} & \hspace{1.65em}\scalebox{1.5}{$\bullet$}\\
    \end{array} & 
    \begin{array}{rcl}
        \scalebox{2}{$\swarrow$}  & \scalebox{2}{$\downarrow$} & \scalebox{2}{$\searrow$} \\[5pt]
        \text{``ca"} \quad& \text{``cb"} & \quad\text{``cc"} \\
        \scalebox{1.5}{$\bullet$}\hspace{1.65em} & \scalebox{1.5}{$\bullet$} & \hspace{1.65em}\scalebox{1.5}{$\bullet$}\\
    \end{array} \\
\end{array}
\end{equation*}

Here's the preview for that:

But, it was kind of difficult to manage the alignments, font size, inverted commas etc in the pyx file itself. I will try to implement it and commit. Thank you.

src/sage/sets/recursively_enumerated_set.pyx

mantepse · 2024-02-14T07:57:59Z

Cool! Could you try to remove the bullet points alltogether?

Also, the arrow labelled 2) must point to an arrow, not to a vertex of the tree.

In fact, either the two arrows labelled 1) and 2) (which "explain" the diagram) should look very different from the other arrows, or should be removed entirely (together with the text "an initial element" and "a function ...").

I would actually vote for removing them: the concept is not all that difficult to understand to warrant such a complicated figure.

src/sage/sets/recursively_enumerated_set.pyx

mantepse · 2024-02-14T08:05:16Z

Great job! Thank you for looking so carefully through the whole text!

Did you use a tool or did you do it by hand?

janmenjayap · 2024-02-14T17:46:33Z

Thank you for your comments :-)

The initial part was done by me; later I used Grammarly to cross-check things. It suggested some modifications like replacing 'depth first search' with 'depth-first search', 'non negative' with 'non-negative' and so on, which I did not appreciate by going through some other documentation of Sage.

Also, I wanted to say that maybe some Sphinx directive like .. raw:: latex (which may at least allow to directly integrate TikZ codes in .pyx/ .py files), .. raw:: html and so, should be enabled in Sage, which may reduce the use of media and allow more flexibility :-)

mantepse · 2024-02-16T09:10:37Z

It would be cool if you could finish this, so it makes it into the upcoming release!

janmenjayap · 2024-02-16T13:23:25Z

Hello,
Apologies for the delay and thank you for your patience.

I have made a new commit with the changes suggested. Just a few observations:

The final image generated looks as follows:
Apparently, \text{``a''} does not act as expected and generates the quotes as follows:
Also, the orientation and length of \swarrow and \searrow in the first set of arrows do not seem quite correct; however, \rotatebox[]()() does not work here as it does in the usual LaTeX math environment.

mantepse · 2024-02-16T15:08:15Z

Thank you!

One question: it might be even better to simplify to single quotes, as one might be used to from working with sage. I.e.,

''
'a'
'ab'

(and without the \hspace)

What do you think?

janmenjayap · 2024-02-16T15:57:30Z

Consider the following:

  \begin{array}{rcl}
      & {^{`}} {^{'}} \\
      \swarrow & \downarrow & \searrow \\[5pt]
      \text{'a'} & \text{`b'} & \text{``c''} \\
      \begin{array}{rcl}
          \swarrow & \downarrow & \searrow \\[5pt]
          ^{``}aa^{"} & ^{`}ab^{'} & `ac' \\
      \end{array} & 
      \begin{array}{rcl}
          \swarrow & \downarrow & \searrow \\[5pt]
          {\scriptstyle `}ba' & ^{\scriptstyle `}bb^{'} & ^{`}bc^{'} \\
      \end{array} & 
      \begin{array}{rcl}
          \swarrow & \downarrow & \searrow \\[5pt]
          ^{`}ca^{'} & ^{`}cb^{'} & ^{`}cc^{'} \\
      \end{array}
  \end{array}

The output is as follows:

Some observations:

Without \hspace it is kind of difficult to customize spacing. I need to check with the different possibilities with \quad, \:, \; and \,.
For single quotes with `', the left one tends to get a bit larger than the right one (see ^{`}ab^{'}) and for backtick, it is required to use ^{`} instead of ` (see `ac' ). Also the use of \scriptstyle does not work in ^{`} (see ^{\scriptstyle `}bb^{'}).
With double quotes {^{``}} {^{"}}, the sizes of the left and right quotes seem similar (see ^{``}aa^{"}), but the same does not seem true for single quotes (see ^{`}ab^{'}).
Use of \text generates desirable results neither for single quotes nor for double quotes (see \text{'a'}, \text{`b'} and \text{``c''} ).
Also, I suppose single quotes are generally used for single characters and double quotes for strings (at least in most of the programming languages, but I might be biased 😅).

mantepse · 2024-02-16T18:57:33Z

I'd go with

\begin{array}{ccc}
      & ``\," \\
      \hfil\swarrow & \downarrow & \searrow\hfil\\
      ``a" & ``b" & ``c" \\
      \begin{array}{ccc}
          \swarrow & \downarrow & \searrow \\
          ``aa" & ``ab" & ``ac" \\
      \end{array} & 
      \begin{array}{ccc}
          \swarrow & \downarrow & \searrow \\
          ``ba" & ``bb" & ``bc" \\
      \end{array} & 
      \begin{array}{ccc}
          \swarrow & \downarrow & \searrow \\
          ``ca" & ``cb" & ``cc" \\
      \end{array}
  \end{array}

It is not all that nice, but it's readable and very simple.

janmenjayap · 2024-02-17T03:59:35Z

Cool.
I have modified accordingly.

mantepse

looks great! I would like to wait for the linter, to make sure that there is no trailing whitespace. Unfortunately, I do not have the rights to approve the workflow, it seems!

Modified the doc in `sets/recursively_enumerated_set.py ` (in the section `Example: Forest structure`) to display the HTML nicely; also corrected some typos.   Sphinx `.. MATH::` directive is unable to render the `picture` environment in LaTeX, as a result of which the image is not begin displayed nicely in [sets/recursively_enumerated_set](https://doc.sagema th.org/html/en/reference/sets/sage/sets/recursively_enumerated_set.html) . I generated the pdf and png of the required image by compiling the LaTeX code and attached the image. The inspiration is from `Figure: The five complete binary trees with four leaves` in [combinat/tutorial](http s://doc.sagemath.org/html/en/reference/combinat/sage/combinat/tutorial.h tml).  This PR resolves an open issue, that is -- sagemath#37084.  The link of the original documentation -- [sets/recursively_enumerated_s et](https://doc.sagemath.org/html/en/reference/sets/sage/sets/recursivel y_enumerated_set.html). ### 📝 Checklist     - [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. - [ ] I have created tests covering the changes. (Not required it seems.) - [x] I have updated the documentation accordingly. ### ⌛ Dependencies  None.  URL: sagemath#37331 Reported by: Janmenjaya Panda Reviewer(s): Martin Rubey

janmenjayap · 2024-02-29T19:59:18Z

It seems that there have been too many commits.
Shall I squash them to a single commit (just for cleanup)?

mantepse · 2024-03-01T08:24:49Z

No, I don't think so.

tscrim

No, I don't think so either. It has useful information of things that have been tried.

mantepse · 2024-03-01T21:36:46Z

@tscrim, did you look at the generated html? I think it looks terrible :-(

I am unhappy about the EXAMPLES: being followed by a heading, and I am unhappy about the example LaTeX - for some reason, the LaTeX - html converter completely messes up the quotes. Maybe we should simply remove them, and use \emptyset for the empty word? (I must say, the example is not all that brilliant, but never mind.)

janmenjayap · 2024-03-02T14:14:40Z

Hello,

I want to make two points.

Regarding the examples, as far as the initial section is concerned, the current doc looks like:

Recursively enumerated set

## Definitions and properties of RES

AUTHORS:

Sébastien Labbé, April 2014, at Sage Days 57, Cernay-la-ville

EXAMPLES:
No hypothesis on the structure

## Definition of No hypothesis on the structure
## Examples of No hypothesis on the structure

Symmetric structure

## Examples of Symmetric structure

Graded structure
## Examples of Graded structure

Forest structure
Forest structure [Ex 1]
## Example 1 of Forest structure
Forest structure [Ex 2]
## Example 2 of Forest structure

This is a temporary solution. I believe we should include the definitions of Symmetric structure, Graded structure and Forest structure and finally it may look like the following:

Recursively enumerated set

## Definitions and properties of RES

AUTHORS:

Sébastien Labbé, April 2014, at Sage Days 57, Cernay-la-ville

No hypothesis on the structure

## Definition of No hypothesis on the structure
EXAMPLES:
## Examples of No hypothesis on the structure

Symmetric structure

## Definition of Symmetric structure
EXAMPLES:
## Examples of Symmetric structure

Graded structure

## Definition of Graded structure
EXAMPLES:
## Examples of Graded structure

Forest structure

## Definition of Forest structure
EXAMPLES:
Forest structure [Ex 1]
## Example 1 of Forest structure
Forest structure [Ex 2]
## Example 2 of Forest structure

This seems like a more robust and better solution concerning the examples than the existing one. This will address both the issues raised by @mantepse and @tscrim and give a better overview of the doc.

Regarding the LaTeX, actually that quotation thing I observed earlier while I generated HTML in my local system and I suggested ^{`}ca^{'} instead of ``ca'' (please take a look at the earlier discussion/ comments on this PR). Yep, either we can try out with the former one (aka ^{`}ca^{'} ) or we may remove the quotations. But, I think the first one is better, as it represents the string structure.

Please consider putting needs work tag as of now :-)
Thank you for this active discussion.

janmenjayap · 2024-03-02T14:19:25Z

Also, the Lint test is failing. I suppose, once again there is some trailing whitespace.
Will fix it (if any).

tscrim · 2024-03-02T17:19:10Z

@mantepse I don’t think it looks that bad, but okay. Let me again suggest keeping how the original version was but changing all of the headings to be .. RUBRIC::. Looking a little more carefully, I don’t think it makes much sense to subhead the examples for the forest structure. Just start and label them one-by-one as .. RUBRIC:: Forest Structure (Example 1) as they are examples.

I don’t think it is useful to duplicate documentation with the (precise) definitions. They are examples. If someone wants the definitions, they can look later in the doc.

I agree with @mantepse that the quotes in the latex looks pretty bad in the html. I would remove them all and replace the empty with $\emptyset$ (\emptyset).

The lint test is very reliable. If it is failing, then it needs to be fixed.

tscrim · 2024-03-04T13:38:43Z

@mantepse What do you think about the doc now?

mantepse · 2024-03-04T14:26:59Z

Looks OK! Possibly the module heading should be Recursively Enumerated Sets, to make it more similar to the rest? (https://deploy-preview-37331--sagemath.netlify.app/html/en/reference/sets/)

janmenjayap · 2024-03-04T16:40:17Z

Is it required to change the file name from recursively_enumerated_set.pyx to recursively_enumerated_sets.pyx and any other such changes that are analogous and possible, concerning recursively enumerated sets?

mantepse · 2024-03-04T16:56:19Z

Is it required to change the file name from recursively_enumerated_set.pyx to recursively_enumerated_sets.pyx and any other such changes that are analogous and possible, concerning recursively enumerated sets?

NO!

github-actions · 2024-03-04T20:52:22Z

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

tscrim · 2024-03-05T12:56:48Z

@mantepse Any additional comments? I am ready to set this to a positive review.

mantepse · 2024-03-05T14:15:49Z

Perfect!

janmenjayap added 2 commits February 14, 2024 12:23

Modified the doc to display the HTML nicely

b9805bb

Corrected a spelling error

7e22d86

janmenjayap mentioned this pull request Feb 14, 2024

fix documentation of sets/recursively_enumerated_set.py #37084

Closed

2 tasks

grhkm21 added p: minor / 4 s: needs work c: documentation labels Feb 14, 2024

Corrected some typos

d39eedc