-
Notifications
You must be signed in to change notification settings - Fork 182
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
Version support and automatic CI build #2214
Changes from all commits
b6bd557
8930562
17f3669
4ae6982
f3bff95
9bcd230
4935935
5c1ff05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||||||||||
name: publish | ||||||||||||||
'on': | ||||||||||||||
push: | ||||||||||||||
branches: | ||||||||||||||
ktp-docs-ci | ||||||||||||||
|
||||||||||||||
jobs: | ||||||||||||||
build: | ||||||||||||||
|
||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||
|
||||||||||||||
steps: | ||||||||||||||
- uses: actions/checkout@v3 | ||||||||||||||
- name: install dependencies | ||||||||||||||
run: | | ||||||||||||||
sudo apt update | ||||||||||||||
sudo apt install -y pandoc git | ||||||||||||||
pip3 install -r requirements-doc.txt | ||||||||||||||
pip3 install scikit-learn-intelex | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had forgotten to comment here but: we'd probably want to change this logic towards either building from source, or downloading artifacts from github - otherwise this wouldn't work out when pushing to release branches (would build docs for the wrong version), nor when trying to build docs for unreleased versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I totally agree. |
||||||||||||||
cd doc | ||||||||||||||
source set_version.sh | ||||||||||||||
echo Generating version $DOC_VERSION | ||||||||||||||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Add it to the github environment to save some work/lines going forward. |
||||||||||||||
- name: build docs | ||||||||||||||
run: | | ||||||||||||||
cd doc | ||||||||||||||
chmod +x build-doc.sh | ||||||||||||||
./build-doc.sh | ||||||||||||||
mv _build/scikit-learn-intelex ../../output | ||||||||||||||
Comment on lines
+25
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Sorry since I am not so knowledgeable on the docs side. If build-doc.sh doesn't have the right permissions, we can set those in the repo to what is necessary (does it hurt to store build-doc.sh as 755 instead)? |
||||||||||||||
- name: deploy docs | ||||||||||||||
run: | | ||||||||||||||
cd doc | ||||||||||||||
source set_version.sh | ||||||||||||||
cd .. | ||||||||||||||
Comment on lines
+31
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
With the previous suggestion these 3 lines are unnecessary as |
||||||||||||||
git fetch | ||||||||||||||
git checkout -- doc/build-doc.sh | ||||||||||||||
git checkout gh-pages | ||||||||||||||
rm -rf $DOC_VERSION | ||||||||||||||
mv -f ../output/$DOC_VERSION . | ||||||||||||||
mv ../output/versions.json . | ||||||||||||||
mv ../output/index.html . | ||||||||||||||
rm -rf doc | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this |
||||||||||||||
git config --global user.name "${GITHUB_ACTOR}" | ||||||||||||||
git config --global user.email "${GITHUB_ACTOR}@github.com" | ||||||||||||||
git add . | ||||||||||||||
git commit -m "latest HTML output" | ||||||||||||||
git push -f https://${GITHUB_ACTOR}:${{secrets.GITHUB_TOKEN}}@github.com/${GITHUB_REPOSITORY}.git HEAD:gh-pages | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: what happens if something goes wrong during the build? For example, we might run into errors such as failing to import the modules from which autodoc generates documentation, which would manifest as a "warning" being issued but some docs still being built. Could there be a check for build warnings that would make this script fail (avoiding pushing anything to GH) if something goes wrong? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any errors in the github workflow will stop the docs from publishing. We could turn on "error on warning" in the sphinx build, if we wanted to gate against any issue in the Sphinx build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that'd be ideal, so that we don't accidentally end up with empty docs. But would be better if it could ignore some warnings selectively - currently there are some warnings when building There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example benign warnings when building
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#example-1-passing-the-github_token-as-an-input let me know if i am missing something, sentence just before this link: "Commits pushed by a GitHub Actions workflow that uses the GITHUB_TOKEN do not trigger a GitHub Pages build." |
||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,13 @@ mkdir $SAMPLES_DIR | |
cd .. | ||
rsync -a --exclude='doc/$SAMPLES_DIR/daal4py_data_science.ipynb' examples/notebooks/*.ipynb doc/$SAMPLES_DIR | ||
|
||
# build the documentation | ||
cd doc | ||
make html | ||
|
||
source ./set_version.sh | ||
export SPHINXPROJ=scikit-learn-intelex | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also have |
||
export BUILDDIR=_build | ||
export SOURCEDIR=sources | ||
|
||
sphinx-build -b html $SOURCEDIR $BUILDDIR/$SPHINXPROJ/$DOC_VERSION | ||
cp versions.json $BUILDDIR/$SPHINXPROJ | ||
echo "<meta http-equiv=\"refresh\" content=\"0; URL='/$SPHINXPROJ/$DOC_VERSION/'\" / >" >> $BUILDDIR/$SPHINXPROJ/index.html |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import subprocess | ||
import os | ||
|
||
def get_version(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @icfaust Could you please look into whether this captures the version number correctly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i would prefer if we got rid of this file and set_version.sh entirely and replaced it with
|
||
major = '' | ||
minor = '' | ||
pip_output = subprocess.run(["pip","list"],capture_output=True,encoding='utf-8') | ||
lines = pip_output.stdout.split("\n") | ||
for line in lines: | ||
if line.startswith("scikit-learn-intelex"): | ||
version = line.split()[1].split(".") | ||
major = version[0] | ||
minor = version[1] | ||
|
||
return major + "." + minor |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export DOC_VERSION=$(python -c "from get_doc_version import get_version; print(get_version())") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
function load_versions(json){ | ||
var button = document.getElementById('version-switcher-button') | ||
var container = document.getElementById('version-switcher-dropdown') | ||
var loc = window.location.href; | ||
var s = document.createElement('select'); | ||
s.style = "border-radius:5px;" | ||
const versions = JSON.parse(json); | ||
for (entry of versions){ | ||
var o = document.createElement('option'); | ||
var optionText = ''; | ||
if ('name' in entry){ | ||
optionText = entry.name; | ||
}else{ | ||
optionText = entry.version; | ||
} | ||
o.value = entry.url; | ||
if (current_version == entry.version){ | ||
o.selected = true; | ||
} | ||
o.innerHTML = optionText; | ||
s.append(o); | ||
} | ||
s.addEventListener("change", ()=> { | ||
var current_url = new URL(window.location.href); | ||
var path = current_url.pathname; | ||
//strip version from path | ||
var page_path = path.substring(project_name.length+current_version.length+3); | ||
window.location.href = s.value + page_path; | ||
}); | ||
container.append(s); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<div class="version-switcher__container dropdown"> | ||
<script> | ||
current_version = "{{current_version}}" | ||
project_name = "{{project_name}}" | ||
</script> | ||
<div id="version-switcher-button" | ||
class="version-switcher__button" | ||
data-bs-toggle="dropdown" | ||
aria-haspopup="listbox" | ||
aria-controls="version-switcher-dropdown" | ||
aria-label="Version switcher list" | ||
style="display:inline-block;padding-left:10px;padding-right:10px;" | ||
> | ||
Choose version <!-- this text may get changed later by javascript --> | ||
<span class="caret"></span> | ||
</div> | ||
<div id="version-switcher-dropdown" | ||
class="version-switcher__menu dropdown-menu list-group-flush py-0" | ||
role="listbox" aria-labelledby="{{ button_id }}"> | ||
<!-- dropdown will be populated by javascript on page load --> | ||
</div> | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
[ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about "stable" version tag in addition to latest? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this file needs to be manually curated, we can add any tags we want to it. |
||
{ | ||
"name": "2025.1 (latest)", | ||
"version": "2025.1", | ||
"url": "/scikit-learn-intelex/2025.1/" | ||
}, | ||
{ | ||
"name": "2025.0", | ||
"version": "2025.0", | ||
"url": "/scikit-learn-intelex/2025.0/" | ||
}, | ||
{ | ||
"version": "2024.6", | ||
"url": "/scikit-learn-intelex/2024.6/" | ||
}, | ||
{ | ||
"version": "2024.3", | ||
"url": "/scikit-learn-intelex/2024.3/" | ||
}, | ||
{ | ||
"version": "2023.2", | ||
"url": "/scikit-learn-intelex/2023.2/" | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come git is necessary?