conda-forge / vtk-feedstock

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

Generate an egg-info file and test the pkg_resources finds it #64

Closed mwcraig closed 6 years ago

mwcraig commented 6 years ago

Checklist

This PR is intended to fix the issue described in https://github.com/enthought/mayavi/issues/652, which occurs because because the conda-forge vtk currently doesn't produce anything that pkg_resources can find.

conda-forge-linter commented 6 years 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.

christianbrodbeck commented 6 years ago

Looks like it needs setuptools added as dependency

===== testing package: vtk-8.1.1-py36hdceb404_202 =====
running run_test.py
Traceback (most recent call last):
  File "/Users/distiller/miniconda3/conda-bld/vtk_1531645356383/test_tmp/run_test.py", line 5, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'
Tests failed for vtk-8.1.1-py36hdceb404_202.tar.bz2 - moving package to /Users/distiller/miniconda3/conda-bld/broken
WARNING:conda_build.build:Tests failed for vtk-8.1.1-py36hdceb404_202.tar.bz2 - moving package to /Users/distiller/miniconda3/conda-bld/broken
removing: vtk-8.1.1-py36hdceb404_202.tar.bz2
TESTS FAILED: vtk-8.1.1-py36hdceb404_202.tar.bz2
Exited with code 1
mwcraig commented 6 years ago

Thanks @christianbrodbeck!

jakirkham commented 6 years ago

Would it make sense to upstream this change?

cc @demarle

mwcraig commented 6 years ago

Any object to merging this, @jakirkham? I don't object at all to it going upstream but would like to get vtk working with mayavi

msarahan commented 6 years ago

I would argue that this should not be upstreamed. vtk as a pip package has an equivalent of this, and that's what mayavi is expecting. The wheels are built completely differently. Because we build vtk as a traditional library, we must add this for usage with things that think it's a python package. This is really a packaging thing, and does not belong in vtk proper.

CC @jcfr, who I discussed this with at SciPy.

jcfr commented 6 years ago

:+1:

May be worth adding a comment in the build script indicating that this is required to support package making use of __requires__ (e.g mayavi). See https://setuptools.readthedocs.io/en/latest/pkg_resources.html#workingset-objects

jcfr commented 6 years ago

:+1:

Cadair commented 6 years ago

can this now be merged? :)

mwcraig commented 6 years ago

Just did it...

jakirkham commented 6 years ago

Friendly note: Normally I try not to merge anything here until the US evening as the builds take several hours to complete, which is a bit rough on queue when people are actively using it. Something to keep in mind.

mwcraig commented 6 years ago

oops; should have thought about that....

grlee77 commented 6 years ago

Unless I am mistaken, builds didn't show up at https://anaconda.org/conda-forge/vtk/files because both this PR and the recently merged #63 bumped the build number to 2. I guess we will need a new PR to bump the build number and rebuild...

jakirkham commented 6 years ago

Ouch! Yep, you're right. This elucidates it.

mwcraig commented 6 years ago

:facepalm: -- @jakirkham do you want the honors of opening the PR to bump the build number or shall I? I think we could open that PR and merge before waiting for CI to complete since all we are doing is bumping the build number and we already know the build passes. Agree?

grlee77 commented 6 years ago

we could open that PR and merge before waiting for CI to complete

yeah, I think you could do that. (potentially waiting to make the PR until the evening/night to reduce the CI load during business hours)