Skip to content

Update headless-gui action in build (fix screenshots) #294

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

Merged
merged 10 commits into from
Dec 15, 2023

Conversation

aganders3
Copy link
Contributor

@aganders3 aganders3 commented Dec 5, 2023

References and relevant issues

Hopefully f i x e s Pretty sure now this fixes both #283 and #285

Description

This configures the headless-gui action to not run the tiling window manager it uses by default. My thinking is the tiling behavior in the window manager is causing issues with certain screenshots. See #283 (comment) for more details.

This also updates the action to a new version with a more common default screen size. This change in bit-depth fixes the messed up magicgui screenshot.

Edit: I'm still not totally clear why the stable docs would look okay but I think this is worth trying.

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Dec 5, 2023

Fingers crossed!
Stable docs are from July so maybe something changed?
Good news is that I'm pretty sure if the build artifact is fine then the deployed will also be fine, so we can test easily.

Edit2: Oh and @aganders3 could you also--optimistically--edit the deploy action?

- name: Build Docs
uses: aganders3/headless-gui@v1
env:
GOOGLE_CALENDAR_ID: ${{ secrets.GOOGLE_CALENDAR_ID }}
GOOGLE_CALENDAR_API_KEY: ${{ secrets.GOOGLE_CALENDAR_API_KEY }}
PIP_CONSTRAINT: ${{ github.workspace }}/napari-repo/resources/constraints/constraints_py3.10_docs.txt
with:
# the napari-docs repo is cloned into a docs/ folder, hence the
# invocation below. Locally, you should simply run make docs
run: make -C docs docs GALLERY_PATH=../examples/

@psobolewskiPhD psobolewskiPhD added maintenance CI, dependencies, and other maintenance bug Something isn't working labels Dec 5, 2023
@psobolewskiPhD psobolewskiPhD added this to the 0.5.0 milestone Dec 5, 2023
@aganders3
Copy link
Contributor Author

aganders3 commented Dec 5, 2023

Good news is that I'm pretty sure if the build artifact is fine then the deployed will also be fine, so we can test easily.

Nice - hopefully.

Bad news is I think I need to push a change and run it again because I think it needs a non-empty string to avoid the defaults 🤦.

edit: and we need to keep waiting because as I am learning about JavaScript and GH-Actions APIs in real-time 🤦🤦.

@aganders3
Copy link
Contributor Author

aganders3 commented Dec 5, 2023

Just pointing out a weird traceback here in a recent build - possibly related but let's see how the docs look before I change anything else (for example trying another WM).
https://github.com/napari/docs/actions/runs/7106959550/job/19347465882#step:8:202

@aganders3
Copy link
Contributor Author

aganders3 commented Dec 5, 2023

Hm, the first screenshot on that page is still messed up, but the rest look better.

I am looking at this build: https://github.com/napari/docs/actions/runs/7106959550

@psobolewskiPhD
Copy link
Member

Yeah, the size is fixed, but the first widget is messed up. So I guess this fixes #285 only at the moment.

@aganders3 aganders3 force-pushed the patch-2 branch 3 times, most recently from 391c66f to bd3775c Compare December 6, 2023 02:40
@aganders3 aganders3 marked this pull request as draft December 6, 2023 03:00
@aganders3
Copy link
Contributor Author

aganders3 commented Dec 6, 2023

Okay actually I think that worked!

@lucyleeow @psobolewskiPhD can you check the latest build?
https://github.com/napari/docs/actions/runs/7109505974

I converted to draft because this will still need a new release on the action.

@psobolewskiPhD
Copy link
Member

yes! Looks correct now in the artifact. I don't see issues in other pages!

@psobolewskiPhD
Copy link
Member

@Czaki I saw the same problems in the artifact from the 0.4.19 docs PR, so I think this will need to be cherry picked over to that branch.

@aganders3 aganders3 marked this pull request as ready for review December 6, 2023 15:14
@aganders3
Copy link
Contributor Author

Okay I'm pretty sure this is good-to-go now using headless-gui@v2. Sorry everyone!

Copy link
Member

@psobolewskiPhD psobolewskiPhD left a comment

Choose a reason for hiding this comment

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

I just checked the latest artifact and everything looks good.

@aganders3 aganders3 changed the title Don't run window manager in headless-gui action during build Update headless-gui action in build (fix screenshots) Dec 6, 2023
@lucyleeow
Copy link
Collaborator

Should we get another review? Not sure who would be best...

@lucyleeow
Copy link
Collaborator

lucyleeow commented Dec 11, 2023

Also I question, I see from here: #283 (comment) it seems that the setup in GH and CircleCI is different.

Would trying to mimic Talleys QT setup action to get consistency between CI workflows be a good idea?

@psobolewskiPhD
Copy link
Member

Also I question, I see from here: #283 (comment) it seems that the setup in GH and CircleCI is different.

Would trying to mimic Talleys QT setup action to get consistency between CI workflows be a good idea?

Not sure if the differences are a) required or b) significant. Either way I think changes to CircleCI workflows are out of scope of this PR, but maybe worth noting in an issue?

@lucyleeow
Copy link
Collaborator

lucyleeow commented Dec 11, 2023

No problem, it is more from a maintenance point of view - I guess we assume they both do the same thing, and if the docs look fine in CircleCI we assume they would look okay when we deploy but they use slightly different workflows? I am no good with Qt and window stuff though. (And I really don't know the ins and outs of our doc CIs)
And yes, further changes are definitely out of scope of this PR.

@psobolewskiPhD psobolewskiPhD merged commit 85636bc into napari:main Dec 15, 2023
@aganders3 aganders3 deleted the patch-2 branch December 15, 2023 19:18
@aganders3
Copy link
Contributor Author

Thanks for merging @psobolewskiPhD 🚀

Note it did not close the two associated bugs automatically, probably because I edited in the "closing words". 🙁

@psobolewskiPhD
Copy link
Member

I just wanted to be sure after deployment. Looks good though:
https://napari.org/dev/guides/magicgui.html

Czaki pushed a commit to napari/napari that referenced this pull request Jan 4, 2024
# Description
This PR updates the build_docs workflow to match the latest napari/docs
workflow
Specifically add concurrency (napari/docs PR:
napari/docs#277 ) and to fix graphical issues
related to the xvfb action ( napari/docs PR:
napari/docs#294 )
Czaki pushed a commit to napari/napari that referenced this pull request Jan 10, 2024
This PR updates the build_docs workflow to match the latest napari/docs
workflow
Specifically add concurrency (napari/docs PR:
napari/docs#277 ) and to fix graphical issues
related to the xvfb action ( napari/docs PR:
napari/docs#294 )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maintenance CI, dependencies, and other maintenance task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem screenshots in "Using magicgui in napari" in dev Widget screenshot in magicgui.md error
4 participants