Skip to content

Migrate docs build to GitHub Actions#5541

Open
LiamConnors wants to merge 30 commits intomainfrom
migrate-to-gh-actions
Open

Migrate docs build to GitHub Actions#5541
LiamConnors wants to merge 30 commits intomainfrom
migrate-to-gh-actions

Conversation

@LiamConnors
Copy link
Member

@LiamConnors LiamConnors commented Mar 17, 2026

Description of change

Updates docs build to use GitHub Actions instead of CircleCI. Needs following PR in graphing-library-docs to be merged also plotly/graphing-library-docs#426

Testing strategy

Tested manually by comparing output of builds for CircleCI vs GitHub Actions

Guidelines

@camdecoster camdecoster self-assigned this Mar 18, 2026
name: doc-html
path: doc/build/html/

- name: Create GitHub App token
Copy link
Contributor

Choose a reason for hiding this comment

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

@LiamConnors Curious why we need to create a new token as part of the workflow — is it because the token is very short-lived?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it has something to do with the fact that this workflow is using a GitHub App rather than an access token.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, that's it. we are using a GitHub App with short-lived tokens

Copy link
Contributor

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

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

Everything seems to be running/passing fine in the action. I'll wait to provide a final review until after you respond to questions.

name: doc-html
path: doc/build/html/

- name: Create GitHub App token
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it has something to do with the fact that this workflow is using a GitHub App rather than an access token.

python check-or-enforce-order.py build/html

- name: Upload HTML docs artifact
uses: actions/upload-artifact@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use v6 if that doesn't break anything. v4 has been deprecated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like there's a v7. Updated it. Thanks

Comment on lines +38 to +42
- name: List installed packages
run: |
cd doc
source .venv/bin/activate
uv pip list
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this step needed? The install step should include the version info.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Removed.


- name: Build HTML docs
env:
MAPBOX_TOKEN: ${{ secrets.MAPBOX_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need a Mapbox token? Haven't we converted to using Maplibre?

Copy link
Member Author

Choose a reason for hiding this comment

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

Pages have Maplibre examples. but because there is still some usage of Mapbox and it hasn't been removed from the package yet, some pages still have an example - https://plotly.com/python/tile-scatter-maps/#mapbox-maps

scikit-image==0.20.0
scikit-learn
scipy==1.9.1
setuptools<=81
Copy link
Contributor

Choose a reason for hiding this comment

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

@LiamConnors Why do we need this upper bound?

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