Check vedo backends and document.

adamltyson commented 4 months ago

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.

paulbrodersen commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

Re-raised the issue on the vedo issue tracker.

adamltyson commented 4 months ago

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

paulbrodersen commented 4 months ago

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

paulbrodersen commented 4 months ago

Issue should be resolved now: https://github.com/marcomusy/vedo/pull/1108.

adamltyson commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.

brainglobe / brainrender

Check vedo backends and document. #335