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

Feature/doc guide #622

Merged
merged 43 commits into from
Feb 8, 2023
Merged

Feature/doc guide #622

merged 43 commits into from
Feb 8, 2023

Conversation

NicolasColombi
Copy link
Collaborator

@NicolasColombi NicolasColombi commented Jan 12, 2023

Changes proposed in this PR:

  1. Fix broken links in tutorials and guides
  2. Fix table of content and wrong heading levels
  3. Fix rendered Table of Content in the Website

How:

  1. Switch from a mix of Html and MarkDown to only MarkDown like this:[My header](#My-header) and ## My header for the table of content and the header respectively.

  2. Add blank line before and after the headings and add 1 blank space after # (Best practice according to Markdown guide)

  3. Rearrange Table of Content so that only the main headers are displayed

PR Author Checklist

PR Reviewer Checklist

1) Fix headings
2) Fix typo in content structure: coniguration --> configuration
1)  add table of content and link for section:
     7. Run a jupyter Notebook on Euler and section
     8. Trouble shooting
2) add blank line before and after the headings
3) add 1 blank space after #
@peanutfun peanutfun changed the base branch from main to develop January 13, 2023 13:53
emanuel-schmid and others added 3 commits January 16, 2023 15:17
Modify file: doc/guide/guide_Euler.ipynb
Possible fix to the wrong outline of the content structure in the website
that shows guide_Euler.ipynb.
Comment on lines 33 to 34
"\n",
"## 1. <a id=\"Const\"> Constants </a>\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the notebook itself it doesn't matter much, but sphinx (which is part of the documentation build) doesn't like too many spaces in a title.

Suggested change
"\n",
"## 1. <a id=\"Const\"> Constants </a>\n",
"## 1. <a id=\"Const\">Constants</a>\n",

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I read in the Markdown guide that it was best to put a blank line before and after the heading # and I thought it might be the reason of the wrong display in the website (Which was not...) Should I remove those blank lines for each #heading ?

Also, in the last commit I switched to only markdown like this:[Access to Euler](#access-to-euler) and ##Access to Euler for the table of content and cell respectively, since @peanutfun suggested that the mix of markdown and html might produce the wrong outline in the website. I just checked and it doesn't seem to change.

The main issue is that the headings with two ## are not displayed, only headings with three ###.
I will try few exploratory commits just to see how it changes when I mess with the table of contents.

Do you have any ideas of a possible reason @emanuel-schmid @peanutfun ?
Thank you!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@emanuel-schmid I changed the headers to this style: # 1. My header, so there should not be the problem of spaces now

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry but I don't quite understand... 😕

Copy link
Collaborator Author

@NicolasColombi NicolasColombi Jan 18, 2023

Choose a reason for hiding this comment

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

At first the structure of the headers were like this:

## 1. <a id=\"link_to_my_header\"> My header name </a>

where the link_to_my_header is written in the content section like this:

[My header name](#link_to_my_header)

Since with Lukas we thought that it might have caused the problem on the website I switched to only markdown like this:

## 1. My header name

and in the content:

[My header name](#my-header-name)

Therefore, the problem you mention of having spaces in the header like this: ## 1. <a id=\"Const\"> Constants </a>\n is indirectly also resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I missed what the "problem on the website" is. Is it that the links are not working?
Are they working after the link notation is removed - or are they just not displayed as links anymore?
If they are just not displayed as links on the website anymore - it's a pity somehow since they used to be working when you opened the notebook in jupyter.

Copy link
Collaborator Author

@NicolasColombi NicolasColombi Jan 18, 2023

Choose a reason for hiding this comment

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

@emanuel-schmid Sorry I wasn't clear with what the problem was, the links worked before and works still now. The problem is (was) that in the website the contents were rendered wrong as you can see here on the top right corner under Content. After the changes it looks right: check here. This was due to the fact that in the jupyter notebook doc/guide/Guide_Euler.ipynb the content section had a wrong header level of ### instead of ##. After switching to level ## the content is rendered right in the website.

Putting all headings aligned.
This reverts commit e8d33f1.
Adding number for headings of level 2: ##
@peanutfun
Copy link
Member

peanutfun commented Jan 17, 2023

I gave it a try myself and could not make it work. I think the issue is that every subheading is placed in its own notebook cell. Maybe we should merge these cells? I would also argue that we do not need a new heading for every single instruction.

Edit: See #622 (comment)

@@ -11,42 +11,51 @@
"cell_type": "markdown",
"metadata": {},
"source": [
"\n",
"### Content\n",
Copy link
Member

Choose a reason for hiding this comment

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

@NicolasColombi Here's the issue: The very first subheading has the wrong level. The contents sidebar only lists the headings with the same level afterwards. This should fix it:

Suggested change
"### Content\n",
"## Content\n",

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@peanutfun Ahh that was it! Thank you Lukas, tomorrow evening I will fix it in all the guides.

@NicolasColombi NicolasColombi marked this pull request as ready for review January 18, 2023 08:49
Change header level of content from ### to ## in
doc/guide/guide_CLIMADA_Tutorial.ipynb.

This change should fix the wrong displayment of contents in the website,
as it did for doc/guide/guide_Euler.ipynb.
NicolasColombi and others added 10 commits January 18, 2023 14:18
Fix broken link in website by adapting the anchor in the
content section of doc/guide/Guide_Euler.ipynb.
Redo table of content of doc/guide/guide_Git_Developement.ipynb.
From html to markdown.
Fix two headings in contents that displayed wrong because they
contained ``.
Remove `` from the heading itself and  not only from the content
Split images and Headers in two different cells. Having them in the
same cell caused the links to not work.
Files changed:

1) guide/Guide_Configuration.ipynb
2) guide/Guide_Euler.ipynb
3) guide/Guide_Git_Development.ipynb
4) tutorial/climada_engine_unsequa.ipynb

Changes:

- Only Mardown and no html
- Change in format of contents
Files changed:

1) guide/Guide_CLIMADA_Tutorial.ipynb
2) guide/Guide_Continuous_Integration_and_Testing.ipynb
3) guide/Guide_Miscellaneous.ipynb
4) guide/Guide_Py_Performance.ipynb
5) guide/Guide_PythonDos-n-Donts.ipynb
6) tutorial/climada_engine_unsequa_helper.ipynb

Changes:

- Only Mardown and no html
- Change in format of content
Files changed:

1) climada_python/README.md
2) doc/tutorial/0_intro_python.ipynb
3) doc/tutorial/climada_engine_CostBenefit.ipynb
4) doc/tutorial/climada_entity_Exposures_polygons_lines.ipynb
@emanuel-schmid
Copy link
Collaborator

Thanks a lot! Are you still working on this @NicolasColombi ?
I'll be happy to review otherwise. But could you please undo the re-plotting in doc/tutorial/climada_engine_unsequa.ipynb - or indicate why we have new plots.
And could you describe the three fixes in some more detail, especially 1 and 3?

@peanutfun
Copy link
Member

Maybe I could jump in here and give a bit more context: The new documentation theme features an automated "Contents" overview of each page on the right hand side. It is built automatically from the headings in the underlying document. When the heading levels are not consistent, this overview is simply wrong. An example:
Screenshot 2023-01-09 143638

Nicolas fixes most of these issues here. Additionally, many tutorials feature self-made table of contents that use HTML links. Since Markdown supports in-document links without HTML, we thought it might be better to have "clean" Markdown and remove the HTML tags.

@NicolasColombi
Copy link
Collaborator Author

NicolasColombi commented Jan 28, 2023

@emanuel-schmid Yes you can review the PR, it should be finished :) I will undo the re-plotting first thing monday morning.

Thanks @peanutfun for the context ;)

@emanuel-schmid emanuel-schmid merged commit 82ee7b0 into develop Feb 8, 2023
@emanuel-schmid
Copy link
Collaborator

🙌 Thanks a lot!

@emanuel-schmid emanuel-schmid deleted the feature/doc_guide branch February 8, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants