search

ansys / pydynamicreporting

Pyansys project for Ansys Dynamic Reporting, a report generator tool.
https://dynamicreporting.docs.pyansys.com/
MIT License
7 stars 1 forks source link

Edits to RST and PY files for rendering documentation #11

Closed PipKat closed 1 year ago

PipKat commented 1 year ago

I marked this as a draft PR so that I can methodically review RST files first. I have now also reviewed the RST and PY files for the API. However, I still have to review the RST files and PY files for the examples.

RobPasMue commented 1 year ago

Hi @PipKat! I took the liberty of retriggering the workflow and solve a Vale issue on this PR. Remember to update your branch locally (i.e. git pull) whenever you get back to this 😄

RobPasMue commented 1 year ago

The doc build warnings were a bit more complex... but managed to solve them!

PipKat commented 1 year ago

Hi @PipKat! I took the liberty of retriggering the workflow and solve a Vale issue on this PR. Remember to update your branch locally (i.e. git pull) whenever you get back to this 😄

Thanks, Roberto. It was a big PR and I expected some issues. I didn't expect you to have to solve them for me. Much appreciated!

RobPasMue commented 1 year ago

Thanks, Roberto. It was a big PR and I expected some issues. I didn't expect you to have to solve them for me. Much appreciated!

No big deal - had some time this morning 😄

PipKat commented 1 year ago

@RobPasMue As I noted to you in a chat after your day's end on Friday, I don't see my push, which has this title "Edits based on reding through the doc rendered in the last push." image

If the branch being out of date is the issue, note that I also can't click "Update branch" and update it successfully. I get this error: Update branch attempt failed Head branch was modified. Review and try the merge again.

Remaining tasks consist only of adding some comments and questions to the changed files and then opening this PR for review. If you get this running, one change you might get in before I start my day on Monday is to fix the breadcrumbs to use this format: documentation version. I read about using html_short_title but didn't know where to put it in the configy.py file.

RobPasMue commented 1 year ago

@RobPasMue As I noted to you in a chat after your day's end on Friday, I don't see my push, which has this title "Edits based on reding through the doc rendered in the last push." image

If the branch being out of date is the issue, note that I also can't click "Update branch" and update it successfully. I get this error: Update branch attempt failed Head branch was modified. Review and try the merge again.

Remaining tasks consist only of adding some comments and questions to the changed files and then opening this PR for review. If you get this running, one change you might get in before I start my day on Monday is to fix the breadcrumbs to use this format: documentation version. I read about using html_short_title but didn't know where to put it in the configy.py file.

On it, thanks @PipKat!

PipKat commented 1 year ago

@margalva and @RobPasMue Not a problem for making this project public, but I was wondering why the README content wasn't reused to reduce future maintenance. This is something to consider doing sometime in the future. Also, many of the "Returns" sections aren't properly formatted. The first line, left aligned with the "Returns" title, should be the data type. The second line, indented four spaces, should be the description. You can look at the PyAEDT doc for guidance.

RobPasMue commented 1 year ago

@RobPasMue I seem to remember that there was some way to make the method signature line always display double quotations around default values instead of single quotation marks. The Python files mostly use double quotes, but the method signature shown in the doc seems to always be using single quotes.

Yeah you can do something like this


"""
        mystr : str
            My string value. By default ``"HELLO"``
"""
margalva commented 1 year ago

@margalva and @RobPasMue Not a problem for making this project public, but I was wondering why the README content wasn't reused to reduce future maintenance. This is something to consider doing sometime in the future. Also, many of the "Returns" sections aren't properly formatted. The first line, left aligned with the "Returns" title, should be the data type. The second line, indented four spaces, should be the description. You can look at the PyAEDT doc for guidance.

@PipKat Are you referring to the README for the github repo and the documentation page? We did it like that on purpose. The idea is that developers will be looking at the content of the README on the github repo, so it'll contain info on how to build from source code and things a developer / contributor might be interested in. The content of the README for the documentation page is targeting the end users who just want to download the package and run with it, without need for details on building it and such.

PipKat commented 1 year ago

@margalva and @RobPasMue Not a problem for making this project public, but I was wondering why the README content wasn't reused to reduce future maintenance. This is something to consider doing sometime in the future. Also, many of the "Returns" sections aren't properly formatted. The first line, left aligned with the "Returns" title, should be the data type. The second line, indented four spaces, should be the description. You can look at the PyAEDT doc for guidance.

@PipKat Are you referring to the README for the github repo and the documentation page? We did it like that on purpose. The idea is that developers will be looking at the content of the README on the github repo, so it'll contain info on how to build from source code and things a developer / contributor might be interested in. The content of the README for the documentation page is targeting the end users who just want to download the package and run with it, without need for details on building it and such.

Yes, @margalva. I was referring to using an "include" to the README in the index for the "Getting started" section. In theory, all of our packages could take the approach that you did. However, most seem to prefer having all information within their client library documentation, so they use an "include" so that the README content (sometimes starting at a certain line in this file) also appears in the "Getting started" section. This way, the information is always available to everyone--and maintenance is reduced.