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 mayavi and vtk #285

Closed
wants to merge 5 commits into from
Closed

add mayavi and vtk #285

wants to merge 5 commits into from

Conversation

msarahan
Copy link
Member

@msarahan msarahan commented Apr 6, 2016

Someone on the Anaconda Support mailing list is requesting these packages. I have not really validated or cleaned up these recipes, but I hope the mailing list person might chip in here. Otherwise, I'll come back and clean these up soon.

@jakirkham
Copy link
Member

Which mailing list? conda-forge's? conda's? Something else?

@msarahan
Copy link
Member Author

msarahan commented Apr 6, 2016

Anaconda support

@jakirkham
Copy link
Member

Thanks for clarifying.

@ivoflipse
Copy link
Contributor

Just in case anyone needs some inspiration, we have working recipes for VTK 7 for Linux and Windows: https://github.com/ClinicalGraphics/conda-recipes/tree/master/vtk

@msarahan
Copy link
Member Author

msarahan commented Apr 7, 2016

Thanks @ivoflipse. I'll merge your recipe in here.

@ccordoba12
Copy link
Contributor

@msarahan, please add me as a maintainer. I created the recipes present in conda-recipes.

There's an open PR in conda-recipes that adds support for Python 3, please try to use that one here too.

@@ -0,0 +1,53 @@
{% version = "4.4.4" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing set

@patricksnape
Copy link
Contributor

I also have made a VTK 7/Mayavi build for Python 2/3 and this looks OK to me, other than the broken meta.yaml blocking the build. Keep in mind that using NMake for windows will probably mean that these builds take multiple hours on Appveyor. Using the multi-core generators I was seeing about 1 hour 20 minute builds on Appveyor, so this may take 2 hours per configuration.

Let me know if there is anything I can do to help get this in.

@grlee77
Copy link
Member

grlee77 commented Apr 14, 2016

Hi @msarahan . I would also very much like to see this one go.

The recipe @ccordoba12 referred to where I worked on Python 3 support is here:
conda-archive/conda-recipes#581

I think you need to pass -DVTK_PYTHON_VERSION=3 to CMAKE for python 3.x. The VTK docs state that it should support 3.2+, but I only tested on python 3.4 and 3.5.

Another flag present in the recipe on conda-recipes to consider. I'm not sure exactly what it does, but apparently Matlplotlib related :)
-DModule_vtkRenderingMatplotlib=ON

VTK is also a dependency for #308 (I think currently it is optional, but may be mandatory in the near future?)

@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly conda-forge-admin automated user.

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

@msarahan
Copy link
Member Author

Thanks @patricksnape @grlee77 and @ccordoba12 - I have added both of you as maintainers here, if that's ok.

I tried to copy the changes @grlee77 made in the conda-recipes PR. Please make sure that I have not missed any.

@msarahan
Copy link
Member Author

@ivoflipse do you want to be a maintainer here, too?

@patricksnape and @ivoflipse I'd like to not add code for the CMake generator here, and instead adapt these recipes once the next conda-build release happens. Thanks to @patricksnape, this logic will all be in conda-build, and it will be much cleaner.

I'm not highly opinionated on this, though, so if these builds really are not happening, I'll opt for the solution builds rather than nmake.

@msarahan
Copy link
Member Author

Failures on CircleCI are because we are missing opengl.

@jakirkham
Copy link
Member

jakirkham commented Apr 15, 2016

Failures on CircleCI are because we are missing opengl.

Shouldn't be. We added that earlier today.

@msarahan
Copy link
Member Author

CMake Error at /opt/conda/envs/_build/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
  Could NOT find OpenGL (missing: OPENGL_gl_LIBRARY OPENGL_INCLUDE_DIR)
Call Stack (most recent call first):
  /opt/conda/envs/_build/share/cmake-3.5/Modules/FindPackageHandleStandardArgs.cmake:388 (_FPHSA_FAILURE_MESSAGE)
  /opt/conda/envs/_build/share/cmake-3.5/Modules/FindOpenGL.cmake:172 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  CMake/vtkOpenGL.cmake:65 (find_package)
  ThirdParty/glew/vtkglew/CMakeLists.txt:3 (include)

Where is it installed? Maybe I need to tell CMake where to look?

@jakirkham
Copy link
Member

jakirkham commented Apr 15, 2016

Yeah, I looked closer, we have the library, but we don't have GLUT so we need freeglut3-dev installed.

Think it is time to cleanup this PR ( #346 ) for yum support and merge it. Of course, we haven't merged the equivalent change for conda-smithy ( conda-forge/conda-smithy#134 ), but we will need to do that too. For now, it is always possible to re-render.

@ocefpaf
Copy link
Member

ocefpaf commented Apr 15, 2016

AppVeyor's log says: Looking for stdint.h - not found.

I don't know how important that is for vtk, but we do have that in the msinttypes package.

@msarahan
Copy link
Member Author

If CMake doesn't barf upon not finding stdint.h, it's probably not mandatory. The only true test is building the software, though. Many projects include code that compensates for the lack of stdint.h.

@ocefpaf
Copy link
Member

ocefpaf commented Apr 15, 2016

If CMake doesn't barf upon not finding stdint.h, it's probably not mandatory. The only true test is building the software, though. Many projects include code that compensates for the lack of stdint.h.

Makes sense.

@jakirkham
Copy link
Member

So, we could easily build freeglut and I think that probably makes more sense here. It is basically like a Qt for OpenGL. Besides we are going to want this on other platforms. This shouldn't be too bad as they support CMake.

@jakirkham
Copy link
Member

I'm trying my hand at freeglut will get back.

@jakirkham
Copy link
Member

^ In progress, suggestions and feedback welcome. See PR ( #373 ).

@jakirkham
Copy link
Member

So, I have a working build of freeglut now, but it is not released yet as it requires additional yum packages. However, if you would like, you could try merging that PR ( #373 ) into this one to see if Linux will now build successfully.

If it does work, that means vtk will also have yum requirements. To add them, simply copy the yum_requirements.txt file from my freeglut recipe into your vtk recipe. As I haven't built mayavi before, I don't know how it works, but if vtk is a hard dependency then mayavi is in the same situation.

The idea behind yum requirements is a bit off topic, but can be seen and demonstrated in this PR ( #346 ). That PR would also be the best place for further discussion on it. Just to give the basic idea, this is the simplest way that @pelson and I came up with to install things from yum as part of a package's build process and allow that information to be trivially transferred from staged-recipes to the feedstocks and used in their builds. It also acts as a useful form of documentation as it specifies additional requirements that users will need that are outside of the scope of a basic docker image as outlined by PEP 513 (manylinux). Admittedly, we are not following the letter of the law with manylinux, but we are trying to avoid including libraries that are not on that list. However, our versions might not be the same and we do not install all packages provided by manylinux (e.g. ncurses).

@jakirkham
Copy link
Member

So, use of yum_requirements.txt has been accepted in staged-recipes. The process by which they are integrated into the final feedstock is still under discussion.

@ivoflipse
Copy link
Contributor

I wouldn't mind being a maintainer for this recipe, but I'm on vacation right now, so I'll only be watching from the sideline.

@grlee77
Copy link
Member

grlee77 commented Apr 18, 2016

I tried these out locally using conda build.

The VTK recipe installed without issue on linux with Python 3.5. For mayavi and Python 3.5, I encountered two problems:

1.) Several of the listed dependencies (apptools, envisage, traits, pyface, traitsui) only have Python 2.7 conda packages in the default channel.
Are you interested in a PR for those in staged-recipes? if so, should each be a separate PR or should they just be added to this PR? The recipes for those are all pretty trivial. The ones I used are available in this branch: https://github.com/grlee77/staged-recipes/tree/mayavi_grl

2.) The move done in build.sh (mv examples $EXAMPLES) fails with the following message:

+ mv examples
mv: missing destination file operand after ‘examples’

Is $EXAMPLES something that should be getting set by conda-build itself or should it be in the recipe somewhere? If I comment out the move I am able to complete the build and run various mayavi examples successfully.

@jakirkham
Copy link
Member

That would be great @grlee77. Could we please get them in a separate PR and cross-ref this PR? Thanks.

@grlee77
Copy link
Member

grlee77 commented Apr 18, 2016

@jakirkham : A single separate PR or one PR for each?

@jakirkham
Copy link
Member

Are they dependent on each other?

@grlee77
Copy link
Member

grlee77 commented Apr 18, 2016

As far as I can tell you can build traits on its own with just python and (optionally) numpy.

For the others, the dependency chain seems to be as follows:
envisage requires apptools which requires traitsui and pyface (all require traits)

@jakirkham
Copy link
Member

Let's do traits in one PR. traitsui and pyface in another. Then apptools in another. Finally envisage in its own. That way when we merge them we don't have to handle missing dependencies.

Though if you don't mind restarting CIs and a longer review we can include them together.

@Korijn
Copy link
Contributor

Korijn commented Apr 25, 2016

When creating #453 I overlooked this PR, sorry about that. Anything I can/should do to help?

@msarahan
Copy link
Member Author

@Korijn if you want to scavenge any changes to VTK here into your PR, I think you're more qualified to make sure this is all working correctly. Let me know when you have examined this recipe, and I'll remove VTK from this PR, preferring your PR instead.

@Korijn
Copy link
Contributor

Korijn commented Jun 29, 2016

Since #453 was merged and provides a VTK packages on Linux, at least the VTK part of this PR has become obsolete. There's a continued effort to support VTK on all platforms (struggling with build timeouts) here: conda-forge/vtk-feedstock#1

@pelson
Copy link
Member

pelson commented Jul 7, 2016

@msarahan - hope you don't mind, but I'm going to go ahead and close this PR. Looks like much of it has been salvaged, but if anybody wants to take mayavi forwards, that would be excellent.

@pelson pelson closed this Jul 7, 2016
@msarahan
Copy link
Member Author

msarahan commented Jul 7, 2016

No problem. Glad it lives elsewhere.

@msarahan msarahan deleted the mayavi branch July 7, 2016 15:34
@grlee77 grlee77 mentioned this pull request Sep 14, 2016
@dfroger dfroger mentioned this pull request Jan 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

10 participants