-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Mayavi #2215
Mayavi #2215
Conversation
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 ( |
Error on Gnu/Linux:
qtconsole has a similar issue, and solve it with |
xvfb is also recommand in mayavi doc, will try this solution |
|
||
about: | ||
home: https://github.com/enthought/mayavi | ||
license: BSD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if this is BSD 2-Clause
or BSD 3-Clause
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I check...
- apptools | ||
- envisage | ||
- pyqt | ||
- setuptools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does mayavi
needs pkg_resources
? If not we can remove setuptools
from the run dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems that yes, pkg_resources is needed
build: | ||
- toolchain | ||
- python | ||
- numpy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if mayavi
relies on numpy
ABI? If so we need to use numpy x.x
here and below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
files tvtk/src/array_ext.c
and tvtk/src/array_ext.pyx
calls Numpy C-API, does it means it relies on numpy ABI?
By the way, seems that mayavi and tvtk could be in two differents packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, You do need to add numpy x.x
then.
By the way, seems that mayavi and tvtk could be in two differents packages.
Maybe you could test the new conda-build
feature that creates multiple packages from a single recipe. (But I would not try that now. I would get this merged first ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc says: "TVTK is meant to be installed as part of the mayavi package.", so it must be ok to have 1 package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed to numpy x.x
@dfroger why the change in |
|
||
source: | ||
git_url: https://github.com/enthought/mayavi.git | ||
git_rev: {{ version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this to:
fn: mayavi-{{ version }}.tar.gz
url: https://github.com/enthought/mayavi/archive/{{ version }}.tar.gz
sha256: 36f688b3ea542e9f8cc0d7faa25e1425723cd00acc8aa640169029f33679ab85
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
entry_points: | ||
- mayavi2 = mayavi.scripts.mayavi2:main | ||
- tvtk_doc = tvtk.tools.tvtk_doc:main | ||
skip: True # [osx or (linux and py34) or win] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious but why are you skipping PY34
? (It does not matter as conda-forge is no longer building packages for Python 3.4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just because its VTK dependency is skipped for linux and py34, I'll add a comment about that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or should I just remove the py34 skip, as conda-forge no longer build for 3.4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case just remove the skip. It will skip in conda-forge anyways, since we are not building Python 3.4, but users will still be able to build it elsewhere if they want to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the py34 skip
The file permission change seems to be an error, I'll restaure it |
@@ -49,6 +49,8 @@ about: | |||
license_family: BSD | |||
license_file: LICENSE.txt | |||
summary: The Mayavi scientific data 3-dimensional visualizers | |||
doc_url: http://docs.enthought.com/mayavi/mayavi | |||
dev_url: https://github.com/simplejson/simplejson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Copy-n-pasta 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops... I fixed dev_url to github (same as home page)
Thanks @dfroger! Good luck with the Windows and OS X builds. |
- python | ||
- numpy x.x | ||
- setuptools | ||
- vtk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this have been pinned too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- python : 2.7 and 3.5 are supported
- vtk: I think yes, we shoud pin it to >= 7.1
run: | ||
- python | ||
- numpy x.x | ||
- vtk ==7.1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it need to be pinned exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think >= 7.1 is enough. I'll propose the change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created conda-forge/mayavi-feedstock#1 to not forget
others mayavi PR: #285, #1581, #2014
This PR disable osx and win for now.
It should works on GNU/Linux