Generate API documentation using Sphinx. #59

AhmedBasem20 · 2024-03-13T06:47:20Z

Describe the changes you have made in this PR

Implemented automated API documentation generation using Sphinx and Github Actions. This process involves two steps:

Executing sphinx-apidoc -o doc src to generate initial API documentation into doc folder after extracting docs from src folder.
Running make html to generate HTML documentation in the doc/_build/html folder.

This setup allows for including manual documentation alongside API documentation. I'll try to integrate the markdown files from the doc folder onto the documentation page. And exclude the src/original file as it doesn't have much documentation there.

Documentation Link

https://ahmedbasem20.github.io/TF2.4_IVIM-MRI_CodeCollection

Link this PR to an issue [optional]

Fixes #50

etpeterson

The docs look good! Just a few things.

.github/workflows/sphinx.yml

doc/conf.py

AhmedBasem20 · 2024-03-18T03:07:43Z

Hi @etpeterson, please check the website again and let me know what you think: https://ahmedbasem20.github.io/TF2.4_IVIM-MRI_CodeCollection
We now have a main page that points to the algorithm analysis figures and the API documentation.

I used this action https://github.com/dawidd6/action-download-artifact to get the Figures artifact uploaded by the analysis.yml workflow. And then included it in the docs.
This solution supersedes #49 .

etpeterson

I haven't tried it yet but this multiple source for the pages site is the major thing.

etpeterson · 2024-03-18T06:01:14Z

.github/workflows/sphinx.yml

@@ -35,16 +35,26 @@ jobs:
        run: |
          pip install -r requirements.txt

+      # Action to download artifacts from a different workflow (analysis.yml)
+      - name: Download figures artifact


This introduces a dependency between workflows that we now need to manage. Is it not possible to upload documentation into one folder on the published page from one workflow and figures from another? Or does it purge the page when a new thing is uploaded?

If we do need to merge the results from multiple workflows to publish, then that publish step should probably be a separate workflow triggered when one or both of the source workflows complete. I looks like that's possible with this and it even allows passing artifacts around, so perhaps that's how it's intended?

Another option would be to add this doc building to the analysis workflow. I'm not too happy with that because it seems like it's two separate things, but maybe that's the publishing workflow?

Is it not possible to upload documentation into one folder on the published page from one workflow and figures from another?

Not sure if I understand this part, can you elaborate more? The sphinx.yml workflow downloads the figures from the last completed analysis workflow. And then publish the website. So this doesn't require the current analysis job to be completed, it captures the figures from the last successful one.

I looks like that's possible with this and it even allows passing artifacts around, so perhaps that's how it's intended?

This will be very useful to keep the website updated with the current figures produced by analysis.yml.
I'm thinking of deploying the website twice: a quick one that takes ~50 seconds and uses the last available figures artifact. And the second one is when the new figures are uploaded. What do you think?

on: push: branches: - 'main' workflow_run: workflows: ["analysis.yml"] types: - completed

This is really good. It would be nice if the upload were a separate workflow, but maybe that's a followup improvement.

What happens in the case of a build failure on either branch?

I think the last thing is it would be nice to use the official github method of accessing data from the other workflows. It's much more verbose but is the official method. Let me know your thoughts.

This is really good. It would be nice if the upload were a separate workflow, but maybe that's a followup improvement.

I did the build and deployment on the same workflow because each step depends on the previous one. Not sure how to further improve this, maybe upload the build folder as an artifact and use it on a separate deployment workflow?

What happens in the case of a build failure on either branch?

If the build fails for any reason the website will be available with the last successful build.

I think the last thing is it would be nice to use the official github method of accessing data from the other workflows. It's much more verbose but is the official method. Let me know your thoughts.

Done!

I did the build and deployment on the same workflow because each step depends on the previous one. Not sure how to further improve this, maybe upload the build folder as an artifact and use it on a separate deployment workflow?

That's what I was thinking, but that's a larger refactor at this point. Maybe a followup PR.

Thinking it through, one downside of the current approach is that the documentation is contingent on a successful analysis run. So if the analysis workflow breaks then the documentation stops getting updated.

Fixed! Now if the analysis fails, the figures step will be skipped and the generation will work.

etpeterson

Looking good, just those small comments.

etpeterson · 2024-03-21T04:52:41Z

.github/workflows/analysis.yml

@@ -5,6 +5,7 @@ on:
    branches:
      - 'main'
      - 'analysis/**'
+      - 'test-docs'


Don't you want wildcards here? Or is there some specific branch called test-docs you have in mind?

Oh sorry, this was a branch that I was debugging in. Removed.

etpeterson · 2024-03-21T04:53:16Z

.github/workflows/sphinx.yml


      - name: Build html
        run: |
          mkdir docs/_static
+          ls


Just for debugging?

etpeterson · 2024-03-21T04:54:41Z

.github/workflows/sphinx.yml

I just now realized it's called sphinx.yml. Probably more descriptive would be documentation.yml or something.

etpeterson · 2024-03-21T04:59:06Z

.github/workflows/sphinx.yml

@@ -35,16 +35,26 @@ jobs:
        run: |
          pip install -r requirements.txt

+      # Action to download artifacts from a different workflow (analysis.yml)
+      - name: Download figures artifact


I did the build and deployment on the same workflow because each step depends on the previous one. Not sure how to further improve this, maybe upload the build folder as an artifact and use it on a separate deployment workflow?

That's what I was thinking, but that's a larger refactor at this point. Maybe a followup PR.

Thinking it through, one downside of the current approach is that the documentation is contingent on a successful analysis run. So if the analysis workflow breaks then the documentation stops getting updated.

etpeterson · 2024-03-22T15:44:32Z

This is looking good. I think it's ready. I just want to confirm with @oliverchampion and @petravanhoudt that we're putting documentation in the right place.

I'm thinking the other PR, should go in first and double check that, then this can be integrated. Is that the order you'd prefer @AhmedBasem20?

AhmedBasem20 · 2024-03-22T16:55:28Z

@etpeterson this pull request should replace #49, as we already generate the figures here but inside the sphinx theme.

etpeterson · 2024-03-25T04:19:15Z

@etpeterson this pull request should replace #49, as we already generate the figures here but inside the sphinx theme.

Ah, I see. Ok, close the other one. Let's give a few more days to suggest locations and we can merge this.

Generate API documentation using Sphinx.

09b0a91

etpeterson requested changes Mar 14, 2024

View reviewed changes

.github/workflows/sphinx.yml Outdated Show resolved Hide resolved

.github/workflows/sphinx.yml Outdated Show resolved Hide resolved

doc/conf.py Outdated Show resolved Hide resolved

AhmedBasem20 added 4 commits March 18, 2024 02:04

Integrate the algorithms figures into the documentation website.

255618c

Add the figures artifact to the docs folder before building the website.

6387588

Fix pdf path

6725843

Use figures artifact generated by analysis.yml workflow

c05d0cc

AhmedBasem20 requested a review from etpeterson March 18, 2024 03:10

etpeterson requested changes Mar 18, 2024

View reviewed changes

Deploy when the new figures are generated.

bbb6358

AhmedBasem20 requested a review from etpeterson March 18, 2024 13:20

AhmedBasem20 added 2 commits March 19, 2024 13:43

use the official download method.

fbbd3f0

Prevent deploying on every push

85b13d7

etpeterson requested changes Mar 21, 2024

View reviewed changes

AhmedBasem20 added 2 commits March 21, 2024 16:30

Generate docs even on analysis workflow failure.

b7d5e2f

rename docs workflow.

ab9354b

AhmedBasem20 requested a review from etpeterson March 21, 2024 20:07

AhmedBasem20 mentioned this pull request Mar 25, 2024

Display figures on a new github pages site. #49

Closed

etpeterson merged commit 5fe5728 into OSIPI:main Mar 29, 2024
12 checks passed

AhmedBasem20 mentioned this pull request Apr 7, 2024

Refactor the docs workflow #61

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate API documentation using Sphinx. #59

Generate API documentation using Sphinx. #59

AhmedBasem20 commented Mar 13, 2024

etpeterson left a comment

AhmedBasem20 commented Mar 18, 2024

etpeterson left a comment

etpeterson Mar 18, 2024

AhmedBasem20 Mar 18, 2024

etpeterson Mar 19, 2024

AhmedBasem20 Mar 20, 2024

etpeterson Mar 21, 2024

AhmedBasem20 Mar 21, 2024

etpeterson left a comment

etpeterson Mar 21, 2024

AhmedBasem20 Mar 21, 2024

etpeterson Mar 21, 2024

etpeterson Mar 21, 2024

etpeterson Mar 21, 2024

etpeterson commented Mar 22, 2024

AhmedBasem20 commented Mar 22, 2024

etpeterson commented Mar 25, 2024

Generate API documentation using Sphinx. #59

Generate API documentation using Sphinx. #59

Conversation

AhmedBasem20 commented Mar 13, 2024

Describe the changes you have made in this PR

Documentation Link

Link this PR to an issue [optional]

etpeterson left a comment

Choose a reason for hiding this comment

AhmedBasem20 commented Mar 18, 2024

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpeterson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpeterson commented Mar 22, 2024

AhmedBasem20 commented Mar 22, 2024

etpeterson commented Mar 25, 2024