GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — simplify CI and install extras by kushalkolar · Pull Request #698 · fastplotlib/fastplotlib

Conversation

kushalkolar commented Jan 17, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

A successfull attempt to do the following: 😄

extras deps in setup.py are simplified, was kinda entangled before
use matrix in CI for notebook vs. no notebook installs
introduce non-imgui tests in the matrix to make sure fpl works without imgui

Also updated to use uplaod-artifacts v4, but for reasons I do not understand it merges the imgui and non-imgui images within the same uploaded zip file even though I specify two separate filenames as specified by their docs. One of the artifacts contains both the imgui and non-imgui screnshots (wtf?) and the other one has only the imgui versions which makes no sense (wtf 2) 🤷‍♂️ . Anyways this is fine for us for now so I don't think we should hold back for this minor quirk of upload-artifacts (they might fix it in a future version sooner than us figuring it out)

closes #678

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          simplify CI, WIP

fda0f56

kushalkolar requested a review from clewis7 as a code owner

January 17, 2025 06:26

kushalkolar added 6 commits

January 17, 2025 01:30


          tweak

37d87db


          jupyter rfb v0.5.0 not on pypi yet

909c8c0


          tweak

4daf503


          twaek

8dd07db


          ok let's make this look ugly

4933a32


          typical bash and sed bleh

765e794

github-actions Bot commented Jan 17, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

📚 Docs preview built and uploaded! https://www.fastplotlib.org/ver/simplify-ci

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar added 3 commits

January 17, 2025 02:02


          ok so github really likes it ugly

e22dd0a


          needs more sed

e2428b2


          see if core dump fixed

2e1002f

kushalkolar commented Jan 17, 2025

Copy link

Copy Markdown

Member Author

oh shit this is actually working 🤯

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar added 12 commits

January 17, 2025 02:22


          imgui and no-imgui matrix for regen screenshots

389c911


          update test utils

c3b346f


          account for imagewidget which needs imgui

b2d6c3b


          modify regen

157bdc7


          tweak

5c4f0c5


          skip imgui required tests when not installed

4e24945


          cleanup


          move check earlier

0facc8e


          more informative artifact upload names


          typo

80072ae


          use upload-artifact v4

0caa458


          add no imgui ground truth imageS

d3f99a1

kushalkolar commented Jan 17, 2025

Copy link

Copy Markdown

Member Author

wow after lots of git actions bash wrangling and sed garbling my idea actually worked 😲 !

@clewis7 this will require a more thorough review XD

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar commented

Jan 17, 2025

View reviewed changes

Comment thread

.github/workflows/ci.yml

Comment on lines -63 to -114

		pytest -v examples
		FASTPLOTLIB_NB_TESTS=1 pytest --nbmake examples/notebooks/
		- uses: actions/upload-artifact@v4
		if: ${{ failure() }}
		with:
		name: screenshot-diffs
		path: \|
		examples/diffs
		examples/notebooks/diffs

		test-build-offscreen:
		name: Test Linux, only offscreen
		runs-on: ubuntu-latest
		timeout-minutes: 30
		if: ${{ !github.event.pull_request.draft }}
		strategy:
		fail-fast: false
		matrix:
		include:
		- name: Test py311
		pyversion: '3.11'
		- name: Test py312
		pyversion: '3.12'
		- name: Test py313
		pyversion: '3.13'
		steps:
		- uses: actions/checkout@v4
		with:
		lfs: true
		- name: Set up Python
		uses: actions/setup-python@v5
		with:
		python-version: ${{ matrix.pyversion }}
		- name: Install llvmpipe and lavapipe for offscreen canvas
		run: \|
		sudo apt-get update -y -qq
		sudo apt-get install --no-install-recommends -y libegl1-mesa-dev libgl1-mesa-dri libxcb-xfixes0-dev mesa-vulkan-drivers xorg-dev
		- name: Install dev dependencies
		run: \|
		python -m pip install --upgrade pip setuptools
		# remove pygfx from install_requires, we install using pygfx@main
		sed -i "/pygfx/d" ./setup.py
		pip install git+https://github.com/pygfx/pygfx.git@main
		pip install -e ".["tests-desktop"]"
		- name: Show wgpu backend
		run:
		python -c "from examples.tests.testutils import wgpu_backend; print(wgpu_backend)"
		- name: fetch git lfs files
		run: \|
		git lfs fetch --all
		git lfs pull
		- name: Test examples

kushalkolar Jan 17, 2025

Copy link

Copy Markdown

Member Author

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.

git is stupid, this entire block is actually the previous non-notebooks test pipeline that came after afterwards

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

kushalkolar commented

Jan 17, 2025

View reviewed changes

Comment thread

.github/workflows/ci.yml

		name: Test Linux
		runs-on: ubuntu-latest
		timeout-minutes: 30
		timeout-minutes: 10

kushalkolar Jan 17, 2025

Copy link

Copy Markdown

Member Author

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.

we can go back to 10 mins now that we don't have python3.10 which required more time due to no imgui wheels existing for it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

clewis7 reacted with laugh emoji

All reactions

😄 1 reaction

kushalkolar commented

Jan 17, 2025

View reviewed changes

Comment thread

.github/workflows/ci.yml

		# create string with one of: tests,imgui,notebook; test,imgui; test,notebook ; tests
		# sed removes trailing comma
		# install fastplotlib with given extras options from above
		pip install -e ".[$(echo "tests,${{ matrix.imgui_dep }},${{ matrix.notebook_dep }}" \| sed -e "s/,\+/,/g" -e "s/,$//")]"

kushalkolar Jan 17, 2025

Copy link

Copy Markdown

Member Author

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.

I copy pasted the sed from stackoverflow

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

clewis7 Jan 20, 2025

Copy link

Copy Markdown

Member

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.

nifty

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          increase jupyter_rfb version

defcbc3

kushalkolar commented Jan 18, 2025

Copy link

Copy Markdown

Member Author

Oh wait I can use bash to exclude the ImageWidget nb for the pytest call when imgui doesn't exists

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar added 8 commits

January 17, 2025 21:50


          use bash find to exclude/include imagewidget nbs depending on imgui e…

65e99d7

…xistance


          test imagewidget gc in nbs only if imgui installed

ad4143b


          regen screenshots nb with and w/o imgui

8679d89


          add nb back to regen matrix

b295d12


          we actually only need 2 variants of regen

cdc100c


          prefix no-imgui for nb screenshots

e27e3f3


          add no-imgui-nb screenshots

de96a23


          python version

6482e27

clewis7 approved these changes

Jan 20, 2025

View reviewed changes

clewis7 left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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.

LGTM, very cool stuff! I like the pattern matching with sed

I will let you merge this one

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

.github/workflows/ci.yml

		# create string with one of: tests,imgui,notebook; test,imgui; test,notebook ; tests
		# sed removes trailing comma
		# install fastplotlib with given extras options from above
		pip install -e ".[$(echo "tests,${{ matrix.imgui_dep }},${{ matrix.notebook_dep }}" \| sed -e "s/,\+/,/g" -e "s/,$//")]"

clewis7 Jan 20, 2025

Copy link

Copy Markdown

Member

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.

nifty

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

setup.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar commented Jan 20, 2025

Copy link

Copy Markdown

Member Author

LGTM, very cool stuff! I like the pattern matching with sed

I wouldn't say I like the sed, it's an ugly garble but useful 😂

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar merged commit 29e3ec4 into main

Jan 20, 2025

kushalkolar deleted the simplify-ci branch

January 20, 2025 19:59

Reviewers

clewis7

clewis7 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add test screenshots without imgui

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

kushalkolar commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kushalkolar commented Jan 17, 2025

Uh oh!

kushalkolar commented Jan 17, 2025

Uh oh!

kushalkolar Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

kushalkolar Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

kushalkolar Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

clewis7 Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

kushalkolar commented Jan 18, 2025

Uh oh!

clewis7 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clewis7 Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kushalkolar commented Jan 20, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kushalkolar commented Jan 17, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot commented Jan 17, 2025 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

clewis7 left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.