Overall doc review - Githubissues

PipKat commented 8 months ago

I reordered the sections in the doc to follow what's recommended in Documenting in the PyAnsys developer's guide. This is a small library with little documentation. Here are my observations:

The README and the "User guide" section contain this sentence: "Some API methods may not work." I understand that this vague sentence serves as a catch-all. However, it provides no useable information. Perhaps one or two examples could be added?
In the "Examples" section, the "Basic examples" section is appearing twice in the left navigation pane. I looked for the cause but couldn't find anything obvious. Also, would someone please add a concluding period to the sentence in the following note, which appears in the doc for each example? I don't know where the source is and have asked several times but gotten no response (or fix)! (Based on Alex's reply, it doesn't look like we will be able to fix this as it is an issue that would have to resolved in the Sphinx Gallery repo.)
In the "API reference" section, "Summary" tables do not include descriptions for attributes and constants. Descriptions for these objects should be added to the Python files.

PipKat commented 8 months ago

@AlejandroFernandezLuces Running pre-commit locally results in no errors. The last commit shouldn't have caused any errors, and all checks passed before it! The only meaningful message is for the failed Build Documentation check, which says that artifact storage quota has been hit. Please advise. I'm marking this PR ready for review, but I'd like to get my API reference changes in the doc and look at them in an HTML artifact before this PR is merged.

AlejandroFernandezLuces commented 8 months ago

@AlejandroFernandezLuces Running pre-commit locally results in no errors. The last commit shouldn't have caused any errors, and all checks passed before it! The only meaningful message is for the failed Build Documentation check, which says that artifact storage quota has been hit. Please advise. I'm marking this PR ready for review, but I'd like to get my API reference changes in the doc and look at them in an HTML artifact before this PR is merged.

It seems that there is some kind of temporary quota error, it should be working now.

AlejandroFernandezLuces commented 8 months ago

The source for that note is in sphinx gallery repo:

https://github.com/sphinx-gallery/sphinx-gallery/blob/31e036cf318bcfd639c8e9c23007d98e120df579/sphinx_gallery/gen_rst.py#L1307-L1326

I can try to fork the repo and fix it, but I don't know if they will accept. I'll let you know 🙂

AlejandroFernandezLuces commented 8 months ago

I'm not sure what you mean about the API reference issue. I downloaded the artifact and everything seems to have the changes you've applied. For example, the exceptions module:

AlejandroFernandezLuces commented 8 months ago

I added descriptions to the modules, as well as a general description for the project. Module details is blank because this project doesn't have any global variable, so it is fine. I don't think it can be removed due to how sphinx generates the API.

Regarding the "Some API methods might not work", @igortg could you give us some guidance on which methods do not work so we can document at least some of them?

PipKat commented 8 months ago

I'm not sure what you mean about the API reference issue. I downloaded the artifact and everything seems to have the changes you've applied. For example, the exceptions module:

@AlejandroFernandezLuces Everything looks fine now! The HTML archive I downloaded must have been before the API changes. I'll clean up the PR description to leave just the few items that are still open.

AlejandroFernandezLuces commented 8 months ago

The source for that note is in sphinx gallery repo:

https://github.com/sphinx-gallery/sphinx-gallery/blob/31e036cf318bcfd639c8e9c23007d98e120df579/sphinx_gallery/gen_rst.py#L1307-L1326

I can try to fork the repo and fix it, but I don't know if they will accept. I'll let you know 🙂

Opened a PR to fix this: https://github.com/sphinx-gallery/sphinx-gallery/pull/1255

PipKat commented 8 months ago

The source for that note is in sphinx gallery repo:

https://github.com/sphinx-gallery/sphinx-gallery/blob/31e036cf318bcfd639c8e9c23007d98e120df579/sphinx_gallery/gen_rst.py#L1307-L1326

I can try to fork the repo and fix it, but I don't know if they will accept. I'll let you know 🙂

I'll assume that this isn't going to happen. It's probably only me that it annoys (but it annoys me on a daily basis)!

MaxJPRey commented 8 months ago

Thank you so much @PipKat for your time and review!

AlejandroFernandezLuces commented 8 months ago

Thanks @PipKat for your work 😄

Everything LGTM, merging this PR 🚀

ansys / pyrocky

Overall doc review #25