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

Edits for overall doc review prior to public release #239

Merged
merged 30 commits into from
Jul 26, 2023

Conversation

PipKat
Copy link
Member

@PipKat PipKat commented Jul 11, 2023

PR for reviewing doc for public release. Lots of edits to follow the Google developer documentation style guide and to ensure the documentation for this library is as consistent as possible with the documentation for other PyAnsys libraries. After you review, accept, and merge, the doc review for public release can be considered complete. However, message strings should be short but complete sentences, with concluding punctuation (virtually always a period). Technical documentation should avoid "please" and exclamation points. I think an issue should be created to review and edit message strings. This PR was too big to risk such edits at this late stage. Lastly, whoever wrote the calc_functions.rst file should be given an award! I'm not sure I've ever seen a file with that many lines before!

@PipKat PipKat changed the title Edits to RST fles Edits for overall doc review prior to public release Jul 11, 2023
doc/source/calc_functions.rst Outdated Show resolved Hide resolved
doc/source/calc_functions.rst Outdated Show resolved Hide resolved
doc/source/calc_functions.rst Outdated Show resolved Hide resolved
@PipKat
Copy link
Member Author

PipKat commented Jul 18, 2023

@RobPasMue and @mariostieriansys If we can get all checks on this PR to pass, I can use the HTML documentation artifact to confirm that I've addressed all of the comments that are currently unresolved. However, I am now getting doc build errors about the files in the imported ens content not following numpydoc standards. I KNOW that they don't follow numpydoc standards. They never did, but the checks all passed last night! The files throwing these errors are for files that Mario told me not to edit. In pyensight\src\ansys\pyensight\core\, this would mean that I haven't edited all the *ens*.py files and the listobj.py file for the Native API reference and Object API reference sections. I have no idea why I started getting all these errors about the docstrings in these modules not following the numpydoc standard!

Next. there are also a couple of formatting errors in the calc_functions.rst file that I can't seem to fix, regardless of how hard I try!

Lastly, I feel that in the examples, the images should appear after the sample code. However, in every attempt I've made to format some files in this way, errors were generated. Sadly, I moved the images back before the code block. However, I personally think placing the sample code before the image would provide a much better user experience.

@PipKat
Copy link
Member Author

PipKat commented Jul 18, 2023

@RobPasMue There are some other display issues in the rendered doc. The Object API reference, which is imported, has truncated descriptions on its landing page. Also, some method descriptions are truncated on the Session page:
image

@RobPasMue
Copy link
Member

Hi @PipKat - let me try and solve the remaining workflow issues :)

@randallfrank
Copy link
Collaborator

@RobPasMue and @mariostieriansys If we can get all checks on this PR to pass, I can use the HTML documentation artifact to confirm that I've addressed all of the comments that are currently unresolved. However, I am now getting doc build errors about the files in the imported ens content not following numpydoc standards. I KNOW that they don't follow numpydoc standards. They never did, but the checks all passed last night! The files throwing these errors are for files that Mario told me not to edit. In pyensight\src\ansys\pyensight\core\, this would mean that I haven't edited all the *ens*.py files and the listobj.py file for the Native API reference and Object API reference sections. I have no idea why I started getting all these errors about the docstrings in these modules not following the numpydoc standard!

Next. there are also a couple of formatting errors in the calc_functions.rst file that I can't seem to fix, regardless of how hard I try!

Lastly, I feel that in the examples, the images should appear after the sample code. However, in every attempt I've made to format some files in this way, errors were generated. Sadly, I moved the images back before the code block. However, I personally think placing the sample code before the image would provide a much better user experience.

I'm a little torn on this one, but perhaps we should submit an issue in the ansys-api-pyensight repo to switch the sub API generation core to generate numpydoc instead of google docstrings. The reason I'm torn is that the work cannot be "complete" because a lot of that content is obtained by introspection into the 'ensight' Python API which uses google docstrings, so there will always be gaps, but we can close them a bit more than they are now.

Aside: 'pyensight' is a remote "work-alike" to the 'ensight' API and it is critical that they share the same effective interfaces as much as possible as users need to be able to run the same scripts (at least at the function level) in both APIs.

@mariostieriansys
Copy link
Collaborator

@RobPasMue @PipKat Hi, where we are with this PR?
Possibly I'd like to merge it soon, so that we can work on eventual following issues soon.

doc/source/calc_functions.rst Outdated Show resolved Hide resolved
doc/source/calc_functions.rst Outdated Show resolved Hide resolved
doc/source/calc_functions.rst Outdated Show resolved Hide resolved
@mariostieriansys
Copy link
Collaborator

@PipKat I left a few comments that should help you fix the PR to at least make it pass.
I have a branch called "mostieri/doc_review_test" where I am not getting these errors anymore. However, the docs build are still failing because of the links to ansysproducthelpqa, to the EnSight doc specifically. The problem is that sphinx is trying to access the link to parse it, but to access it it would need to login to the Ansys Help, and of course it cannot. So either we suppress that or we just remove the reference, and we leave just the link as a text

@PipKat PipKat linked an issue Jul 25, 2023 that may be closed by this pull request
@PipKat
Copy link
Member Author

PipKat commented Jul 25, 2023

@mariostieriansys @randallfrank @ansys/pyansys-core
Accepting and merging this PR constitutes the completion of the doc review required for public release of the PyEnSight library. Because this is the first repository that I've worked in that didn't require an approval before merging a PR, I'm going to leave the merging to @mariostieriansys or @randallfrank. I've copied the PyAnsys core team because Roberto is now on vacation, and I want them to be aware that the merging of this PR should complete their tech review. Note also that I created PyEnSight Issue #250 with the few doc cleanup tasks that should be considered for some time after public release of this PyAnsys library. Thanks all!

@mariostieriansys
Copy link
Collaborator

@PipKat that's a good point. We didn't put a required approval at the beginning to speed up the process (and because we were just a few) but probably before releasing we must put in a policy

@MaxJPRey
Copy link

Thanks @PipKat for the review!

@PipKat PipKat merged commit 3686758 into main Jul 26, 2023
@PipKat PipKat deleted the doc/review_for_release branch July 26, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix broken links in documentation
7 participants