Fixing JSON Schema not Rendering Properly in the Documentation #1594

DhruvSondhi · 2021-05-29T15:09:12Z

This PR aims to fix the JSON Schema parsing error present in the Documentation.

Documentation : https://dhruvsondhi.github.io/tardis/branch/jsonschema_documentation/using/components/configuration/configuration.html#configuration-schema

Description

The documentation contains references to the JSON Schema. This is not correctly parsed by the sphinx jsonschema extension & hence raises many warnings.
This PR restructures the JSON Schema to make it possible to render these in the documentation.
Testing needs to be done.

Motivation and context

This will allow for easier linking of the schema in the Documentation. Will allow easier access for the schema to new users

How has this been tested?

Testing pipeline.
Other.

Type of change

Bug fix.
New feature.
Breaking change.
None of the above.

Checklist

My change requires a change to the documentation.
- I have updated the documentation accordingly.
- (optional) I have built the documentation on my fork following the instructions.
I have assigned and requested two reviewers for this pull request.

codecov · 2021-05-29T15:26:24Z

Codecov Report

Merging #1594 (dcc3fed) into master (2bba59c) will not change coverage.
The diff coverage is n/a.

❗ Current head dcc3fed differs from pull request most recent head 06a5f23. Consider uploading reports for the commit 06a5f23 to get more accurate results

@@           Coverage Diff           @@
##           master    #1594   +/-   ##
=======================================
  Coverage   67.20%   67.20%           
=======================================
  Files          73       73           
  Lines        6150     6150           
=======================================
  Hits         4133     4133           
  Misses       2017     2017

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bba59c...06a5f23. Read the comment docs.

jaladh-singhal

This makes page much clear than before. It will be great if you can also introduce proper hierarchy in "Model" section. The TOC after addressing my comments will be:

Model

Abundances
- File Abundance
- Uniform Abundance
Structure
- File Structure
- Specific Structure
  - Densities
    - Branch85_w7 Density
    - Exponential Density
    - Power_law Density
    - Uniform Density

docs/using/components/configuration/configuration.rst

jaladh-singhal · 2021-05-31T17:56:15Z

docs/using/components/configuration/configuration.rst

@@ -74,7 +90,6 @@ The rest of the section can be used to configure uniform abundances for all shel
 relative abundance fraction. If it does not add up to 1., TARDIS will warn --- but normalize the numbers.


L69-L74 (in current file) will also go up, right after L58 (in your changed file) - they together explain about abundances.

Sure, Didn't know how they linked. I will move it to the requested place 😅

Sorry @jaladh-singhal, Can you please re-iterate this change once I push the updated file?

I don't see them moved up. You just need to Ctrl+X this content from "..warning:: ..... normalize the numbers" and Ctrl+V right after "The abundance section ..... a file containing the abundance information."

docs/using/components/configuration/configuration.rst

DhruvSondhi · 2021-06-01T08:31:40Z

On the same lines, do we need to restructure the abstracts in the Montecarlo Convergence Strategies?

jaladh-singhal · 2021-06-03T15:58:10Z

On the same lines, do we need to restructure the abstracts in the Montecarlo Convergence Strategies?

Actually that needs to be rewritten because convergence_criteria no longer exists (most probably it needs to be changed to convergence_strategy) and the paragraph ends incompletely - I'll create an issue once this is merged it needs to be done by someone with physics background.

isaacgsmith · 2021-06-03T16:01:55Z

On the same lines, do we need to restructure the abstracts in the Montecarlo Convergence Strategies?

Actually that needs to be rewritten because convergence_criteria no longer exists (most probably it needs to be changed to convergence_strategy) and the paragraph ends incompletely - I'll create an issue once this is merged it needs to be done by someone with physics background.

That's related to some work I will be doing soon, so I am going to be looking into that.

jaladh-singhal

Thanks for making changes - there are few things still left.

Besides, (it should not be implemented in this PR) but sphinx-rtd-theme has very small font-size differences from h4 to h6 making it visually hard to distinguish these heading levels - maybe we can override corresponding CSS in config.

jaladh-singhal · 2021-06-03T16:05:21Z

docs/using/components/configuration/configuration.rst

@@ -74,7 +90,6 @@ The rest of the section can be used to configure uniform abundances for all shel
 relative abundance fraction. If it does not add up to 1., TARDIS will warn --- but normalize the numbers.


I don't see them moved up. You just need to Ctrl+X this content from "..warning:: ..... normalize the numbers" and Ctrl+V right after "The abundance section ..... a file containing the abundance information."

docs/using/components/configuration/configuration.rst

jaladh-singhal · 2021-06-03T16:15:00Z

docs/using/components/configuration/configuration.rst

+Densities
+"""""""""


Actually, densities should be a subsection of "Specific Structure" (as you can compare my proposed TOC with your rendered TOC after these changes) - I know it seems awkward but is it possible to somehow nest it within specific structure schema (L75)'s title?

This seems to be difficult as we are on <H6> at the moment on those heading. I don't think we can move it under the section 🤔 . What are your opinions, @jaladh-singhal ?
https://documentation-style-guide-sphinx.readthedocs.io/en/latest/style-guide.html#headings ---> Reference to the heading hierarchy

I have it set up so that the Density section is under the Structure Section but it seems so that it is not being generated correctly 🤔

Actually, the jsonschema extension is creating the appropriate headings based on the schema that I have linked. I don't think I can make it so that the Density subsection comes under the Specific Structure section because the Specific Structure is at <H6> & Density is also at <H6>.

@DhruvSondhi I checked it in web inspector before commenting that:

Structure is at H4

File Structure is at H5

Densities is at H5

Branch85_w7 Density is at H6

Other density types also at H6

And Overview TOC also shows same heirarchy that's why

I'm not sure how you're checking it but they are not all at H6 - I'll say to use web inspector in browser dev tools.

The actual problem here is how .. jsonschema:: renders and how rst format allows us to specify nesting in headings - both are leading to same heading levels. It seems to me that jsonschema when rendered, its title (i.e. Specific Structure) takes one next heading level in the section in which it is placed (i.e. Structure with H4) - so it becomes H5. And, the different underline characters in "Densities" as compared to "Structure" makes rst renderer think that Densities is subheading in Structure hence also making it H5.

That's why I said if it is possible to specify that "Densities" is subheading to jsonschema "Specific Structure"?

Since @smithis7 will break down this configuration page into multiple pages as part of his doc restructure project - we can leave it for now. Hence merging - thanks for addressing other 2 comments.

Then I think so the document I linked pertaining to the heading hierarchy in rst is wrong or I may be doing something wrong 🤔

…s-sn#1594) * Fixing JSON Schema not rendering in the documentation * Restructuring the configurational abstract * Moving the documentation part to relevant sections

wkerzendorf requested review from wkerzendorf and jaladh-singhal May 30, 2021 00:15

wkerzendorf approved these changes May 30, 2021

View reviewed changes

jaladh-singhal requested changes May 31, 2021

View reviewed changes

jaladh-singhal requested a review from isaacgsmith May 31, 2021 18:24

DhruvSondhi force-pushed the jsonschema_documentation branch from 474451b to 00e1c4d Compare June 1, 2021 04:57

DhruvSondhi requested a review from jaladh-singhal June 1, 2021 08:27

DhruvSondhi force-pushed the jsonschema_documentation branch from 9086b0e to 68b2a41 Compare June 1, 2021 09:02

andrewfullard added the documentation label Jun 1, 2021

isaacgsmith approved these changes Jun 1, 2021

View reviewed changes

DhruvSondhi force-pushed the jsonschema_documentation branch from 68b2a41 to 6112d5b Compare June 2, 2021 04:55

jaladh-singhal requested changes Jun 3, 2021

View reviewed changes

DhruvSondhi added 2 commits June 3, 2021 22:37

Fixing JSON Schema not rendering in the documentation

2d1f71b

Restructuring the configurational abstract

f07a88b

DhruvSondhi force-pushed the jsonschema_documentation branch from fe4d618 to 03ebcfe Compare June 3, 2021 17:08

Moving the documentation part to relevant sections

06a5f23

DhruvSondhi force-pushed the jsonschema_documentation branch from 03ebcfe to 06a5f23 Compare June 3, 2021 17:26

jaladh-singhal approved these changes Jun 3, 2021

View reviewed changes

jaladh-singhal merged commit 44765a4 into tardis-sn:master Jun 3, 2021

DhruvSondhi deleted the jsonschema_documentation branch June 3, 2021 19:17

isaacgsmith mentioned this pull request Jun 10, 2021

Restructure Documentation #1643

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing JSON Schema not Rendering Properly in the Documentation #1594

Fixing JSON Schema not Rendering Properly in the Documentation #1594

DhruvSondhi commented May 29, 2021 •

edited

codecov bot commented May 29, 2021 •

edited

jaladh-singhal left a comment •

edited

jaladh-singhal May 31, 2021

DhruvSondhi Jun 1, 2021 •

edited

DhruvSondhi Jun 1, 2021

jaladh-singhal Jun 3, 2021

DhruvSondhi commented Jun 1, 2021

jaladh-singhal commented Jun 3, 2021

isaacgsmith commented Jun 3, 2021

jaladh-singhal left a comment

jaladh-singhal Jun 3, 2021

jaladh-singhal Jun 3, 2021

DhruvSondhi Jun 3, 2021 •

edited

DhruvSondhi Jun 3, 2021

DhruvSondhi Jun 3, 2021

jaladh-singhal Jun 3, 2021 •

edited

jaladh-singhal Jun 3, 2021 •

edited

DhruvSondhi Jun 3, 2021 •

edited

		@@ -74,7 +90,6 @@ The rest of the section can be used to configure uniform abundances for all shel
		relative abundance fraction. If it does not add up to 1., TARDIS will warn --- but normalize the numbers.

		Densities
		"""""""""

Fixing JSON Schema not Rendering Properly in the Documentation #1594

Fixing JSON Schema not Rendering Properly in the Documentation #1594

Conversation

DhruvSondhi commented May 29, 2021 • edited

codecov bot commented May 29, 2021 • edited

Codecov Report

jaladh-singhal left a comment • edited

Choose a reason for hiding this comment

jaladh-singhal May 31, 2021

Choose a reason for hiding this comment

DhruvSondhi Jun 1, 2021 • edited

Choose a reason for hiding this comment

DhruvSondhi Jun 1, 2021

Choose a reason for hiding this comment

jaladh-singhal Jun 3, 2021

Choose a reason for hiding this comment

DhruvSondhi commented Jun 1, 2021

jaladh-singhal commented Jun 3, 2021

isaacgsmith commented Jun 3, 2021

jaladh-singhal left a comment

Choose a reason for hiding this comment

jaladh-singhal Jun 3, 2021

Choose a reason for hiding this comment

jaladh-singhal Jun 3, 2021

Choose a reason for hiding this comment

DhruvSondhi Jun 3, 2021 • edited

Choose a reason for hiding this comment

DhruvSondhi Jun 3, 2021

Choose a reason for hiding this comment

DhruvSondhi Jun 3, 2021

Choose a reason for hiding this comment

jaladh-singhal Jun 3, 2021 • edited

Choose a reason for hiding this comment

jaladh-singhal Jun 3, 2021 • edited

Choose a reason for hiding this comment

DhruvSondhi Jun 3, 2021 • edited

Choose a reason for hiding this comment

DhruvSondhi commented May 29, 2021 •

edited

codecov bot commented May 29, 2021 •

edited

jaladh-singhal left a comment •

edited

DhruvSondhi Jun 1, 2021 •

edited

DhruvSondhi Jun 3, 2021 •

edited

jaladh-singhal Jun 3, 2021 •

edited

jaladh-singhal Jun 3, 2021 •

edited

DhruvSondhi Jun 3, 2021 •

edited