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

tutorials: make CORAL example consistent #272

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

wihobbs
Copy link
Member

@wihobbs wihobbs commented Aug 7, 2024

Problem: the CORAL interactive tutorial uses the username "elvis" in all places but one.

Solution: make the document consistent by removing the last reference to "hobbs17" and replacing with "elvis".

Copy link
Member

@jameshcorbett jameshcorbett left a comment

Choose a reason for hiding this comment

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

🥇

Copy link
Member

@cmoussa1 cmoussa1 left a comment

Choose a reason for hiding this comment

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

LGTM!

Problem: the CORAL interactive tutorial uses the username
"elvis" in all places but one.

Solution: make the document consistent by removing the last
reference to "hobbs17" and replacing with "elvis".
@wihobbs
Copy link
Member Author

wihobbs commented Aug 9, 2024

After running flux start make html I get this diff:

The HTML pages are in _build/html.
hobbs@hobbs-mbp:~/flux-docs$ git status
On branch update-elvis
Your branch is up to date with 'origin/update-elvis'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   auto_examples/auto_examples_jupyter.zip
	modified:   auto_examples/auto_examples_python.zip
	modified:   auto_examples/index.rst

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	sg_execution_times.rst

no changes added to commit (use "git add" and/or "git commit -a")
hobbs@hobbs-mbp:~/flux-docs$ git diff auto_examples/index.rst
diff --git a/auto_examples/index.rst b/auto_examples/index.rst
index be13a05..24218aa 100644
--- a/auto_examples/index.rst
+++ b/auto_examples/index.rst
@@ -22,6 +22,7 @@ of using the Flux Python API.
 
     <div class="sphx-glr-thumbnails">
 
+.. thumbnail-parent-div-open
 
 .. raw:: html
 
@@ -40,6 +41,8 @@ of using the Flux Python API.
     </div>
 
 
+.. thumbnail-parent-div-close
+
 .. raw:: html
 
     </div>

I don't understand why elvis should be causing this, and I'm not comfortable adding compressed zipped folders to a PR until I understand what they're doing. The underlying files do not differ:

hobbs@hobbs-mbp:~/flux-docs$ diff auto_examples/example_job_submit_api.py ../example_job_submit_api.py 
hobbs@hobbs-mbp:~/flux-docs$ diff auto_examples/example_job_submit_api.ipynb ../example_job_submit_api.ipynb 

@wihobbs
Copy link
Member Author

wihobbs commented Aug 9, 2024

you could say I don't "git" it
edit: i'll see myself out

@cmoussa1
Copy link
Member

cmoussa1 commented Aug 9, 2024

I tried restarting the failed job to see if it would generate any kind of specific error:

Testing that you built the auto-examples, if this fails run flux start make html locally.
  😢️ Auto examples were not updated! Different size of index.rst:
  1497 vs. 1557

I wonder why index.rst is being affected by the change you made in corals.rst?

@wihobbs
Copy link
Member Author

wihobbs commented Aug 9, 2024

I don't think these problems have anything to do with any changes:

hobbs@hobbs-mbp:~/flux-docs$ git status
**HEAD detached at upstream/master**
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   auto_examples/auto_examples_jupyter.zip
	modified:   auto_examples/auto_examples_python.zip
	modified:   auto_examples/index.rst

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	sg_execution_times.rst

no changes added to commit (use "git add" and/or "git commit -a")

That's off of HEAD at upstream/master.

Also worth noting that @jameshcorbett wasn't hitting this problem 2 days ago. Could the sphinx-gallery v0.17.1 release from 3 days ago have anything to do with this? When I looked more closely at the flux start make html commands, that's the only thing that occurred to me, although rolling back the version didn't solve the problem.

@wihobbs
Copy link
Member Author

wihobbs commented Aug 9, 2024

Hmm. Interesting. My working theory is that some change was made to sphinx-gallery v0.17.1 that requires these new tags to make the gallery render.

@wihobbs
Copy link
Member Author

wihobbs commented Aug 9, 2024

@vsoch, added you as an approving reviewer since we've had some back and forth about the auto-examples.

@jameshcorbett
Copy link
Member

Also worth noting that @jameshcorbett wasn't hitting this problem 2 days ago.

Not two days ago--I merged the PR two days ago but the action ran four months ago (it was an old PR I forgot to merge).

Copy link
Member

@jameshcorbett jameshcorbett left a comment

Choose a reason for hiding this comment

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

🥇

Problem: sphinx-gallery has rolled some updates
in the last few months. As a working theory, one of
these updates might have required a new div in the
auto-generated gallery we use for displaying examples.

Update that gallery's index.rst to get the CI working
again.
@wihobbs wihobbs added merge-when-passing mark PR for auto-merging by mergify.io bot and removed merge-when-passing mark PR for auto-merging by mergify.io bot labels Aug 12, 2024
@mergify mergify bot merged commit 8d822fb into flux-framework:master Aug 12, 2024
7 checks passed
@wihobbs
Copy link
Member Author

wihobbs commented Aug 12, 2024

Whoops. Ok, well, if we need to go back and fix we can. I don't think this really did anything drastic.

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