overhaul pg_rst.py to dump proper *.rst files#17
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the RST generation pipeline in pg_rst.py and its templates to produce more “Sphinx-friendly” .rst output (notably by moving TOC construction into generated content and updating link/anchor behavior), and bumps the package version.
Changes:
- Updates
index.rst.tempto rely on a generated__TOC__block instead of a static.. toctree::template. - Refactors
PgRST.create_toc()to emit a.. toctree::directive and attempts to add per-section/local TOCs. - Adjusts internal link formats/anchors (sections/examples) and bumps project version to
2.0.6.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/rda_python_miscs/rst_templates/section.rst.temp |
Updates template header metadata for generated section pages. |
src/rda_python_miscs/rst_templates/index.rst.temp |
Simplifies index template and delegates TOC rendering to generated __TOC__. |
src/rda_python_miscs/pg_rst.py |
Core logic changes for TOC generation, linking, list rendering, and section rendering flow. |
pyproject.toml |
Version bump to 2.0.6. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ms = re.match(r'^\s+-\s+(.*)', line) | ||
| if ms: | ||
| content += "#. " + self.replace_option_link(ms.group(1), secid, 1) + "\n" | ||
| content += "*. " + self.replace_option_link(ms.group(1), secid, 1) + "\n" |
There was a problem hiding this comment.
In create_table(), numbered list items should use #. (or explicit numbers) for each list item. *. is not valid reStructuredText list syntax and will render incorrectly.
| content += "*. " + self.replace_option_link(ms.group(1), secid, 1) + "\n" | |
| content += "#. " + self.replace_option_link(ms.group(1), secid, 1) + "\n" |
| # Title : index_rst.temp | ||
| # Author : Zaihua Ji, zji@ucar.edu | ||
| # Date : 03/17/2026 | ||
| # Purpose : template file for help document index.rst (reStructuredText) | ||
| # | ||
| # Work File : $DSSHOME/lib/templates/index.temp | ||
| # Github : https://github.com/NCAR/rda-python-mics.git |
There was a problem hiding this comment.
The header comment’s GitHub URL appears to have a typo ("rda-python-mics" vs this repo/package name "rda_python_miscs" / "rda-python-miscs"). Also the "Title" comment says index_rst.temp but the actual filename is index.rst.temp. Please update these header metadata lines to match the real file/repo.
| for opt in section['opts']: | ||
| content += self.create_option(opt, secid) | ||
|
|
||
| content += create_toc(section) # add a local TOC for the section and its subsections |
There was a problem hiding this comment.
create_section() calls create_toc(section) without self., which will raise NameError (and also bypasses the class method). Call self.create_toc(section) instead.
| content += create_toc(section) # add a local TOC for the section and its subsections | |
| content += self.create_toc(section) # add a local TOC for the section and its subsections |
| link = "{}`{} <mode_>`_{}".format(pre, opt, after) | ||
| else: | ||
| link = "{}`{} <section{}_>`_{}".format(pre, opt, secid, after) | ||
| link = "{}`{} <section{}>`_{}".format(pre, opt, secid, after) |
There was a problem hiding this comment.
This link target uses <section{}> whereas other internal references in this module use the named-target form <..._> (see the option-link branch just above). As written, this will be treated as a URI-like relative link rather than a reference to the .. _section<id>: target. Please make the target consistent (or switch to Sphinx :ref:/:doc: roles).
| link = "{}`{} <section{}>`_{}".format(pre, opt, secid, after) | |
| link = "{}`{} <section{}_>`_{}".format(pre, opt, secid, after) |
| for section in self.sections: | ||
| if title == section['title']: | ||
| return "`{} <section{}.rst>`_".format(title, section['secid']) | ||
| return "`{} <section{}>`_".format(title, section['secid']) | ||
|
|
There was a problem hiding this comment.
get_title_link() now returns a link with target <sectionN> instead of the named-target form <sectionN_>. This likely won’t resolve to the .. _sectionN: label and may produce a broken relative URL. Please align this with the rest of the module’s internal link strategy (e.g., sectionN_ or Sphinx :ref:).
| # Title : section_rst.temp | ||
| # Author : Zaihua Ji, zji@ucar.edu | ||
| # Date : 03/17/2026 | ||
| # Purpose : template file for help document section.rst (reStructuredText) | ||
| # | ||
| # Work File : $DSSHOME/lib/templates/section.temp | ||
| # Github : https://github.com/NCAR/rda-python-mics.git |
There was a problem hiding this comment.
The header comment’s GitHub URL appears to have a typo ("rda-python-mics" vs this repo/package name "rda_python_miscs" / "rda-python-miscs"). Also the "Title" comment says section_rst.temp but the actual filename is section.rst.temp. Please update these header metadata lines to match the real file/repo.
| from rda_python_common.pg_file import PgFile | ||
| from rda_python_common.pg_util import PgUtil | ||
|
|
||
| from posix import WIFCONTINUED |
There was a problem hiding this comment.
The import from posix import WIFCONTINUED appears unused (no references in this module) and also makes the module non-portable on non-POSIX platforms. Please remove it unless it’s required elsewhere.
| from posix import WIFCONTINUED |
| clevel = section['level'] if csection else 0 | ||
| csecid = section['secid'] if csection else "" |
There was a problem hiding this comment.
create_toc() uses section['level'] / section['secid'] when csection is provided, but section is not defined at that point (will raise NameError). Use csection for these values (or rename variables to avoid confusion with the later loop variable).
| clevel = section['level'] if csection else 0 | |
| csecid = section['secid'] if csection else "" | |
| clevel = csection['level'] if csection else 0 | |
| csecid = csection['secid'] if csection else "" |
| # appendix A: list of examples for the parent section and its subsections | ||
| example = "" | ||
| idx = 1 # used as example index | ||
| for example in self.examples: | ||
| opt = example['opt'] | ||
| option = self.options[opt] | ||
| secid = option['secid'] | ||
| content += "- `A.{}. {} Option -{} (-{}) <section{}.rst#e{}>`_\n".format( | ||
| idx, option['type'], opt, option['name'], secid, idx) | ||
| if not psecid or secid.startswith(psecid + "."): | ||
| example += "- `A.{}. {} Option -{} (-{}) <{}_e{}>`_\n".format( | ||
| idx, option['type'], opt, option['name'], secid, idx) | ||
| idx += 1 |
There was a problem hiding this comment.
In create_toc(), example is initialized as a string and then reused as the loop variable (for example in self.examples), so example += ... will fail at runtime. Also psecid is referenced but never defined. Please rename the accumulator (e.g., examples_content) and use the intended section id filter variable (likely csecid).
No description provided.