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

Check vedo backends and document. #335

Open
adamltyson opened this issue Apr 18, 2024 · 13 comments
Open

Check vedo backends and document. #335

adamltyson opened this issue Apr 18, 2024 · 13 comments
Labels
enhancement New feature or request

Comments

@adamltyson
Copy link
Member

On @paulbrodersen's machine, setting vedo.settings.default_backend = "vtk" is necessary to use brainrender. We should investigate why this is and make sure it's documented properly if others see the same behaviour.

@adamltyson adamltyson added the enhancement New feature or request label Apr 18, 2024
@adamltyson adamltyson mentioned this issue Apr 18, 2024
6 tasks
@paulbrodersen
Copy link
Contributor

paulbrodersen commented Apr 18, 2024

My conda environment specification, in case that helps. It's a YAML file, but I had to convert it to a text file to upload.

environment.txt

@paulbrodersen
Copy link
Contributor

vedo --info
# vedo version      : 2024.5.1  (https://vedo.embl.es)             
# vtk version       : 9.3.0
# numpy version     : 1.23.5
# python version    : 3.10.14 | packaged by conda-forge | (main, Mar 20 2024, 12:45:18) [GCC 12.3.0]
# python interpreter: /home/paul/src/miniconda3/envs/brainrender_dev/bin/python3.10
# installation point: /home/paul/src/miniconda3/envs/brainrender_dev/lib/python3.10/site-packages/v
# system            : Linux 5.4.0-148-generic posix x86_64
# k3d version       : 2.16.1

@paulbrodersen
Copy link
Contributor

For some reason, the default backend is "2D" in my case.

In [1]: import vedo

In [2]: vedo.settings.default_backend
Out[2]: '2d'

@paulbrodersen
Copy link
Contributor

Hah, found the smoking gun!

I have been running code inside an ipython REPL, as I have been constantly inspecting brainrender objects and reading doc strings to wrap my head around how everything works together. Under these conditions, import vedo; print(vedo.settings.default_backend) yields '2d'.

However, iff I run code from the shell, i.e. outside the REPL, the backend defaults to 'vtk':

python -c "import vedo; print(vedo.settings.default_backend)"
# vtk

@paulbrodersen
Copy link
Contributor

Re-raised the issue on the vedo issue tracker.

@adamltyson
Copy link
Member Author

Thanks for digging into this @paulbrodersen. Whatever the outcome is, this should be documented on the brainrender side.

@paulbrodersen
Copy link
Contributor

Looks like the problem is due to a too aggressive Jupyter notebook detection and is getting resolved on vedo's side.

@paulbrodersen
Copy link
Contributor

Issue should be resolved now: marcomusy/vedo#1108.

@adamltyson
Copy link
Member Author

@paulbrodersen thanks for looking into this. What do you think of this solution:

  • Assuming that the backend will be set correctly, and so removing any code that sets it from examples (it may just be the Line actor one).
  • Documenting that it's possible for this need setting (and how to do so)

@paulbrodersen
Copy link
Contributor

I am paranoid, and people don't read documentation, so I would 1) have my own notebook detection that is triggered during import; 2) import vedo and check that it is using the right backend; 3) raise a warning if it is not with instructions on what steps will likely resolve the issue.

@adamltyson
Copy link
Member Author

Do we have any reason to believe that vedo is not doing this correctly (now) or that brainrender would be able to do it any better? I'm hesitant to add code to brainrender that really should be in (or already is in) vedo.

@paulbrodersen
Copy link
Contributor

Do we have any reason to believe that vedo is not doing this correctly (now) or that brainrender would be able to do it any better?

No, and maybe?

Vedo is not numpy/scipy/matplotliblib, so my level of trust is lower and having a bit of defensive code around it seems like a worthwhile investment. Like so many libraries, it has been written and is being maintained by an army of one. And while the core vedo library code looks pretty decent, the project doesn't have CI, and the test suite seems to be patchy (at best) and require a human to check the results. My PR got merged within minutes, so I doubt it's been tested manually on more than one platform. (None of this is meant to be criticism of the vedo maintainer; it's just one of the realities of unpaid OSS development. Most of my libraries could do with better tests as well.)

I also think that this issue is particularly problematic, as no errors are raised when the backend is setup incorrectly. Most users won't even know where to start trouble-shooting. I also happen to maintain a data visualisation library, so a backend issue was one of my first guesses. But I doubt that will be the standard response.

Normally, one would have a regression test to ensure that a solved issue doesn't creep back in again. However, this issue can only be detected at runtime, so I don't see a fail-safe alternative to my proposal above. Of course, everyone has to pick their battles, and it might not be worth your time and effort to double-check that vedo is working as intended -- your call (but you did ask me for my opinion).

@adamltyson
Copy link
Member Author

I spent a while typing out a response to this, in which I stated that I thought it best to not add anything, and rely on this issue being fixed in vedo (thanks for that!).

However, in typing it out I changed my mind, so what I think is best is to:

  • Remove any code setting the backend from the examples
  • Add some logic to check the backend that vedo is using. Raise a warning if we think it's not correct, and explain to the user the implications of the current backend and how to correct it if necessary.
  • Update this docs page to explain the backend issue a bit better, add a section about IPython, and link to the relevant vedo docs.

@adamltyson adamltyson changed the title Document vedo backends Check vedo backends and document. May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants