Skip to content

Conversation

@amgebauer
Copy link
Member

Following @gilrrei comment in #1505 (comment)

@gilrrei What do you think using ruaml to preserve formatting and comments for the input file?

I replaced pyyaml with our fast load_yaml for all other cases.

@amgebauer amgebauer requested review from Copilot and gilrrei November 19, 2025 14:37
Copilot finished reviewing on behalf of amgebauer November 19, 2025 14:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces PyYAML with ruamel.yaml to preserve comments and formatting in YAML documentation files. The changes include:

  • Using ruamel.yaml with the ruamel.yaml.string plugin for rendering YAML in documentation
  • Using the fast load_yaml utility from four_c_common_utils.io for metadata loading where comment preservation is not needed
  • Updating dependencies in build-requirements.txt

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
utilities/four_c_python/src/four_c_documentation/render.py Replaced PyYAML imports with ruamel.yaml for input file loading and dumping; switched metadata loading to use the fast load_yaml utility
utilities/four_c_python/build-requirements.txt Replaced pyyaml dependency with ruamel.yaml and ruamel.yaml.string packages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"""Returns a string of the yaml file in the given filetype.
As of now I can return markdown and restructuredText code blocks.
"""
yaml = YAML(typ=["safe", "string"])
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The typ parameter should be a single string value, not a list. For the dump_to_string method to work, you need to use typ='safe' since ruamel.yaml.string is installed in the requirements. The correct syntax is:

yaml = YAML(typ='safe')

The list syntax typ=["safe", "string"] is not valid for the YAML constructor.

Suggested change
yaml = YAML(typ=["safe", "string"])
yaml = YAML(typ="safe")

Copilot uses AI. Check for mistakes.
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.

1 participant