conda-forge / vtk-feedstock

A conda-smithy repository for vtk.
BSD 3-Clause "New" or "Revised" License
13 stars 64 forks source link

Build web modules for trame #258

Closed banesullivan closed 1 year ago

banesullivan commented 1 year ago

Checklist

This adds the web modules so that this version of VTK is compatible with trame

cc @jourdain

conda-forge-linter commented 1 year ago

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 (recipe) and found it was in an excellent condition.

jourdain commented 1 year ago

The only option that should be needed should be -DVTK_GROUP_ENABLE_Web:STRING=WANT

jourdain commented 1 year ago

But to be clear that GROUP will actually do the same thing as enabling WebCore, WebGLExporter and WebPython. So if the logic is to rather pick modules by hand, the current change-set is correct.

banesullivan commented 1 year ago

I'll leave it up to the maintainers of this feedstock on whether to use the meta -DVTK_GROUP_ENABLE_Web:STRING=WANT flag or the combination I've included here.

Also. The MacOS builds are failing for what looks like an unrelated issue AFAICT

banesullivan commented 1 year ago

@conda-forge/vtk, a review would be greatly appreciated so we could get this merged!

jourdain commented 1 year ago

+1

Tobias-Fischer commented 1 year ago

Seems like this is missing a runtime dependency on numpy?

  File "/Users/runner/miniforge3/conda-bld/vtk_1666106840321/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/lib/python3.10/site-packages/vtkmodules/web/utils.py", line 2, in <module>
    import numpy as np
ModuleNotFoundError: No module named 'numpy'
jourdain commented 1 year ago

Hum weirdly enough it seems that no vtk module require numpy while I always thought that it was not optional.

$ ack vtk_required_python_modules

CMakeLists.txt
414:  get_property(vtk_required_python_modules GLOBAL
415:    PROPERTY  vtk_required_python_modules)
416:  if (vtk_required_python_modules)
417:    list(REMOVE_DUPLICATES vtk_required_python_modules)
419:  string(REPLACE ";" "\n" vtk_required_python_modules "${vtk_required_python_modules}")
421:    "${vtk_required_python_modules}\n")

Rendering/Matplotlib/CMakeLists.txt
19:    vtk_required_python_modules "matplotlib>=2.0.0")

Web/Python/CMakeLists.txt
23:    vtk_required_python_modules "wslink>=1.0.4")

Not sure if we want to add numpy to the list of Web module dependency [Web/Python/CMakeLists.txt]?

Tobias-Fischer commented 1 year ago

For the sake of this pull request, simply adding numpy to the run requirements should be sufficient. However agreed that fixing it properly upstream will make sense, too.

banesullivan commented 1 year ago

I just added numpy here to the run requirements to hopefully get this to pass. @jourdain and I will have to open an MR upstream in VTK to add numpy as a dependency

I'm thinking wslink may need to be added as a dependency here which might require making a feedstock for it... https://pypi.org/project/wslink/

Tobias-Fischer commented 1 year ago

Unless there are objections, I'm happy to merge if all builds succeeds - will do in ~24 hours.

mathstuf commented 1 year ago

Hum weirdly enough it seems that no vtk module require numpy while I always thought that it was not optional.

Everything that wants it checks for existence first, so there's no requirement. There is the vtk[numpy] feature request in the wheel to make it happen though.

Tobias-Fischer commented 1 year ago

@mathstuf - I don't think this is the case. In https://github.com/Kitware/VTK/blob/master/Web/Python/vtkmodules/web/utils.py numpy is imported without checking for existence, and https://github.com/Kitware/VTK/blob/master/Web/Python/CMakeLists.txt does not list numpy as python dependency.

Tobias-Fischer commented 1 year ago

I'm thinking wslink may need to be added as a dependency here which might require making a feedstock for it... https://pypi.org/project/wslink/

Good catch. I guess we should wait merging this PR till there is a wslink conda package, and then add it as a run dependency here.

banesullivan commented 1 year ago

wslink is on it's way: https://github.com/conda-forge/staged-recipes/pull/20806

mathstuf commented 1 year ago

Well, getting code that, when included, assumes numpy to declare that should definitely be fixed then…

banesullivan commented 1 year ago

I've added both numpy and wslink as run dependencies here but IMO they should both be hard host dependencies

Tobias-Fischer commented 1 year ago

Sure. I guess they should be in both host and run?

mathstuf commented 1 year ago

AFAIK, VTK's own build doesn't care about numpy at all unless the test suite is enabled.

banesullivan commented 1 year ago

Right, but I think it's worth including since I assume almost every user installing VTK from conda is trying to use the Python bindings and numpy dataset interface in https://github.com/Kitware/VTK/blob/f19f9769410a09e5964a217ba33ed58522909eea/Wrapping/Python/vtkmodules/numpy_interface/dataset_adapter.py#L64-L68 etc

For the sake of purity, I'm happy to leave these dependencies as run and not host -- thus not requiring their installation on users machines. However, I think this is a recipe for confusion.

banesullivan commented 1 year ago

Further, I'd love for this recipe to move closer to wheel the on PyPI. Both in dependencies and in enabled modules

Tobias-Fischer commented 1 year ago

I’d be happy to merge this PR as is and deal with the other things in another PR?

jourdain commented 1 year ago

I think we are good for that to be merged as-is for now. Thanks @Tobias-Fischer !

FanDuan commented 1 year ago

I am new at pyvista and vtk and it is hard for me to understand these. Is there an easy way to deal with this problem?

banesullivan commented 1 year ago

If install the latest vtk from conda-forge or PyPI, this should "just work", nothing for you to do as a user

FanDuan commented 1 year ago

It works! Thank you very much.

shariqfarooq123 commented 1 year ago

So How do I solve this error? I want interactivity inside Jupyter notebooks. I am using VSCode remote.

Your build of VTK does not have the proper web modules enabled.
These modules are typically enabled by default with the
`-DVTK_GROUP_ENABLE_Web:STRING=WANT` build flag.

Conda users: This is a known issue with the conda-forge VTK feedstock.
See https://github.com/conda-forge/vtk-feedstock/pull/258

Falling back to a static output.

Looks like it is leading me to this page but installing from conda-forge or PyPI doesn't "just work". @banesullivan

banesullivan commented 1 year ago

@shariqfarooq123, please open a new issue in PyVista about this. You'll need to include a pyvista.Report there too

https://github.com/pyvista/pyvista/issues/new/choose

lkaly commented 1 year ago

Thanks very much! When I install the conda-forge, the version of vtk is 9.1.0, and it still has this problem. I install the vtk package from PyPI, the version of vtk is 9.2.6, it woks.

ycebear commented 1 year ago

An experience, just for your information:

When I installed PyVista in February 2023 with pip install pyvista[all] (win10) everything was fine a (GREAT THANKY YOU to all the folks implementing PyVista!). When I did the same in these days on a different win10-computer nothing worked.... (a long list with trials and error messages). Now I'm back in the race with the following installation procedure (with conda nothing worked):

  1. pip install pyvista[all]==0.38.6
  2. pip uninstall vtk
  3. pip uninstall trame
  4. pip install vtk==9.2.6
  5. pip install trame==2.4.0
  6. if necessary write in the code: np.bool = np.bool_

And here the informations about the versions :

IPython          : 8.12.2
ipykernel        : 6.25.0
ipywidgets       : 7.8.0
jupyter_client   : 6.1.12
jupyter_core     : 5.3.0
jupyter_server   : 1.23.4
jupyterlab       : 3.5.3
nbclient         : 0.5.13
nbconvert        : 6.5.4
nbformat         : 5.7.0
notebook         : 6.5.4
qtconsole        : not installed
traitlets        : 5.7.1
Windows-10-10.0.22621-SP0
AMD64

python 3.9.17 (main, Jul  5 2023, 21:22:06) [MSC v.1916 64 bit (AMD64)]

numpy 1.24.3
matplotlib 3.7.1
vtk 9.2.6
trame 2.4.0
pyvista 0.38.6