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

feat(opentrons-ai-server): convert rst files to markdown file #15703

Merged
merged 16 commits into from
Jul 29, 2024

Conversation

syao1226
Copy link
Collaborator

@syao1226 syao1226 commented Jul 18, 2024

re AUTH-541

Overview

Converting an HTML file, which was converted from Python API documents (RST format files), to a markdown file.

Test Plan

  • Ensures a new build/docs/html/v2 folder containing an index.html file is created under the utils folder
  • Ensures the final markdown file has a version-aware filename and is created in the api/data folder
  • Ensure that the API version reference section has been removed from the main markdown file and written to a markdown file created in the api/data folder

Changelog

  • add python script under opentrons-ai-server/api/utils folder to handle markdown conversion
  • create data folder to store output markdown files
  • add BeautifulSoup and markdownify to pipfile

Review requests

Risk assessment

@syao1226 syao1226 requested a review from Elyorcv July 18, 2024 16:30
@syao1226 syao1226 requested review from a team as code owners July 18, 2024 16:30
@koji
Copy link
Contributor

koji commented Jul 19, 2024

need to add BeautifulSoup and markdownify to pipefile.

@koji
Copy link
Contributor

koji commented Jul 19, 2024

I recommend you export the output to a specific folder not utils directly.
something like utils/markdown or utils/output.

@Elyorcv
Do we need to keep the converted file on monorepo?

@koji
Copy link
Contributor

koji commented Jul 19, 2024

need to add BeautifulSoup and markdownify to pipefile.

@Elyorcv they would be under [dev-packages]?

@koji koji changed the title feat(opentrons_ai_server): convert rst files to markdown file feat(opentrons-ai-server): convert rst files to markdown file Jul 22, 2024
@koji
Copy link
Contributor

koji commented Jul 22, 2024

If there isn't any specific reason, we should add test for the new code.

@koji koji requested a review from y3rsh July 22, 2024 21:08
@koji
Copy link
Contributor

koji commented Jul 22, 2024

This PR get check/js error because the generated markdown-file doesn't follow our formatting-rule. This is another reason that I don't think that the generated markdown file is needed to be in the mono repo.

@Elyorcv
Copy link
Contributor

Elyorcv commented Jul 23, 2024

I recommend you export the output to a specific folder not utils directly. something like utils/markdown or utils/output.

@Elyorcv Do we need to keep the converted file on monorepo

A Markdown file is just a different version of RST. So I think it is better to keep it in the monorepo.
But better location should be decided. For now we my create a folder named data under opentrons-ai-server/api and save there @koji @syao1226

@Elyorcv
Copy link
Contributor

Elyorcv commented Jul 23, 2024

This PR get check/js error because the generated markdown-file doesn't follow our formatting-rule. This is another reason that I don't think that the generated markdown file is needed to be in the mono repo.

why dont we format according to check/js? @syao1226

Copy link
Contributor

@Elyorcv Elyorcv left a comment

Choose a reason for hiding this comment

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

Looking good. If we add tests then all good to go

Comment on lines 200 to 202
html_file_path = os.path.join(current_dir, "build", "docs", "html", "v2", "index.html")
markdown_file_path = os.path.join(current_dir, "..", "data", f"python_api_{current_version}.md")
reference_file_path = os.path.join(current_dir, "..", "data", "api_version_reference.md")
Copy link
Contributor

Choose a reason for hiding this comment

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

How about if we change like so:
python_api_219_reference.md
python_api_219_docs.md

def get_markdown_format() -> None:
"""Generates a version-aware Markdown file from HTML documentation."""
current_version = get_latest_version()
command = "pipenv run sphinx-build -b singlehtml ../api/docs/v2 api/utils/build/docs/html/v2"
Copy link
Contributor

Choose a reason for hiding this comment

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

please change paths with os.path.join.

Copy link
Contributor

@Elyorcv Elyorcv left a comment

Choose a reason for hiding this comment

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

LGTM, well done Shiyao!

@syao1226 syao1226 closed this Jul 29, 2024
@syao1226 syao1226 reopened this Jul 29, 2024
@syao1226 syao1226 merged commit eca20ab into edge Jul 29, 2024
8 checks passed
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