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

Build web modules for trame #258

Merged
merged 7 commits into from
Oct 21, 2022
Merged

Build web modules for trame #258

merged 7 commits into from
Oct 21, 2022

Conversation

banesullivan
Copy link
Contributor

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

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

cc @jourdain

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

recipe/bld.bat Outdated Show resolved Hide resolved
@jourdain
Copy link

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

@jourdain
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@jourdain
Copy link

+1

@Tobias-Fischer
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@mathstuf
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor Author

wslink is on it's way: conda-forge/staged-recipes#20806

@mathstuf
Copy link

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

@banesullivan
Copy link
Contributor Author

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

@Tobias-Fischer
Copy link
Contributor

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

@mathstuf
Copy link

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

@banesullivan
Copy link
Contributor Author

banesullivan commented Oct 21, 2022

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
Copy link
Contributor Author

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

@Tobias-Fischer
Copy link
Contributor

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

@jourdain
Copy link

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

@FanDuan
Copy link

FanDuan commented Feb 9, 2023

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
Copy link
Contributor Author

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

@FanDuan
Copy link

FanDuan commented Feb 9, 2023

It works! Thank you very much.

@shariqfarooq123
Copy link

shariqfarooq123 commented May 13, 2023

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
Copy link
Contributor Author

@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
Copy link

lkaly commented May 24, 2023

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
Copy link

ycebear commented Aug 14, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants