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

Add fastplotlib #27629

Merged
merged 10 commits into from
Feb 4, 2025
Merged

Add fastplotlib #27629

merged 10 commits into from
Feb 4, 2025

Conversation

hmaarrfk
Copy link
Contributor

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/fastplotlib/meta.yaml) and found it was in an excellent condition.

Copy link
Contributor

Hi! This is the staged-recipes linter and your PR looks excellent! 🚀

@hmaarrfk
Copy link
Contributor Author

I'll circle bback in a few weeks,

@kushalkolar
Copy link
Member

v0.3.0 just released

recipes/fastplotlib/meta.yaml Outdated Show resolved Hide resolved
recipes/fastplotlib/meta.yaml Outdated Show resolved Hide resolved
@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 28, 2025

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/fastplotlib/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/fastplotlib/meta.yaml:

  • ℹ️ PyPI default URL is now pypi.org, and not pypi.io. You may want to update the default source url.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13025114374. Examine the logs at this URL for more detail.

@kushalkolar
Copy link
Member

need to force WGPU_OFFSCREEN=1 for the import test, rendercanvas is looking for a backend.

@hmaarrfk
Copy link
Contributor Author

need to force WGPU_OFFSCREEN=1 for the import test, rendercanvas is looking for a backend.

it checks for that at import time? do you have run code at import time? could we disable that?

@kushalkolar
Copy link
Member

need to force WGPU_OFFSCREEN=1 for the import test, rendercanvas is looking for a backend.

it checks for that at import time? do you have run code at import time? could we disable that?

fpl exposes rendercanvas loop during import, and rendercanvas looks for a backend. Let me trying setting offscreen canvas

- pygfx ~=0.7.0
- cmap

test:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
test:
test:
script_env:
- WGPU_FORCE_OFFSCREEN=1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah didn't work unfortunately.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be my guess as well. Or rather RENDERCANVAS_FORCE_OFFSCREEN (WGPU_FORCE_OFFSCREENdoes the same thing but is backwards compat).

Could it be that it failed for a different reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

To expand a bit: if one of these env vars is set, it should not be able to reach the raising of the exception that the builds currently fail with.

Copy link
Member

Choose a reason for hiding this comment

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

might have to set the env var for the entire yaml not just the test, not sure if that yaml keyword is valid for conda-forge

@hmaarrfk can you give me write access to your fork, I can iterate faster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no i unfortnately can't do that since I use this fork alot and I have super powers at conda-forge.

You are free, if you want to just copy all that I did and submit it as your own work (I give you @kushalkolar the right to do this with no attribution).

Tag me for a review, I can help.

But generally a few ways forward:

  1. Change the behavior of fastplotlib -- i understsand why you might not have time to do this now.
  2. Write a run_test.py file that just set the os.environ key you need: https://github.com/conda-forge/ipython-feedstock/blob/a8d52e4bf5029876b98e2d9c116252f9f4082e2b/recipe/run_test.py

the run_test.py will give you more control over how the tests are run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK so i ended up doing this:

===== testing package: fastplotlib-0.3.0-pyhd8ed1ab_0 =====
running run_test.py
/home/conda/staged-recipes/build_artifacts/fastplotlib_1738640761567/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/lib/python3.10/site-packages/fastplotlib/__init__.py:29: RuntimeWarning: WGPU could not enumerate any adapters, fastplotlib will not work.
This is caused by one of the following:
1. You do not have a hardware GPU installed and you do not have software rendering (ex. lavapipe) installed either
2. Your GPU drivers are not installed or something is wrong with your GPU driver installation, re-installing the latest drivers from your hardware vendor (probably Nvidia or AMD) may help.
3. You are missing system libraries that are required for WGPU to access GPU(s), this is common in cloud computing environments.
These two links can help you troubleshoot:
https://wgpu-py.readthedocs.io/en/stable/start.html#platform-requirements
https://fastplotlib.readthedocs.io/en/latest/user_guide/gpu.html

  warn(
0.3.0
===== fastplotlib-0.3.0-pyhd8ed1ab_0 OK =====

seems the tests are OK right?

Copy link
Member

Choose a reason for hiding this comment

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

yup! looks gtg

hmaarrfk and others added 6 commits January 31, 2025 18:12
Co-authored-by: Kushal Kolar <kushalkolar@gmail.com>
Co-authored-by: Kushal Kolar <kushalkolar@gmail.com>
Co-authored-by: Kushal Kolar <kushalkolar@gmail.com>
Co-authored-by: Kushal Kolar <kushalkolar@gmail.com>
@hmaarrfk
Copy link
Contributor Author

fpl exposes rendercanvas loop during import

Just FYI: This kind of initialization during import typically makes it hard to integrate within other projects. This might be the first issue I open after I start to play with it!

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 31, 2025

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/fastplotlib/meta.yaml) and found some lint.

Here's what I've got...

For recipes/fastplotlib/meta.yaml:

  • ❌ The test section contained an unexpected subsection name. script_env is not a valid subsection name.

For recipes/fastplotlib/meta.yaml:

  • ℹ️ PyPI default URL is now pypi.org, and not pypi.io. You may want to update the default source url.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13082321591. Examine the logs at this URL for more detail.

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/fastplotlib/meta.yaml) and found it was in an excellent condition.

@hmaarrfk
Copy link
Contributor Author

https://github.com/fastplotlib/fastplotlib/blob/d4af1a900a8d7c89bcc25601badf1bf1a9a92ff2/fastplotlib/utils/gui.py#L11 is pretty annoying to happen at import time.

I get you are writing an application, and you can do that, but IMO the "library" aspect of FPL should not be imposing this.

@kushalkolar
Copy link
Member

fpl isn't an application, it's definitely meant to just be a lib

@almarklein any ideas here? We could move the auto canvas backend selection to when a figure is created for the first time 🤔

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Feb 1, 2025

I would just try to just remove that entire file and see if your code still works….

@almarklein
Copy link
Contributor

From what I remember about this, is that fastplotlib prefers/preferred qt, because it used GUI elements that were either qt/ipywidgets. So if a glfw widget was created (the default for rendercanvas) it could not display such widgets.

I have not kept track very well, but I've seen a lot of imgui in fastplotlib, so maybe the qt-specific code can be removed? Then this GUI pre-selection can also be removed.

Otherwise, we could perhaps think of a way to prefer qt using an environment variable.

@kushalkolar
Copy link
Member

kushalkolar commented Feb 3, 2025

It's actually because we import loop from rendercanvas.auto upon import in fastplotlib. https://github.com/pygfx/rendercanvas/blob/main/rendercanvas%2Fauto.py#L203

That's where the error in this workflow is coming from.

A bit busy now but will try playing with it this later in the week.

@hmaarrfk hmaarrfk marked this pull request as ready for review February 4, 2025 04:46
@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Feb 4, 2025

ok excited to try this out. thanks you two!

@hmaarrfk hmaarrfk merged commit 0b5aed5 into conda-forge:main Feb 4, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants