djfun / audio-visualizer-python

a little GUI tool to render visualization videos of audio files
Other
242 stars 58 forks source link

setup.py broken in feature-newgui branch #45

Closed AsavarTzeth closed 7 years ago

AsavarTzeth commented 7 years ago

Since commit https://github.com/djfun/audio-visualizer-python/commit/e92e9d79f95ad67e83074ef318278c3486601eac setup.py is completly borked.

It tried fixing it myself but ran into other issues.

My attempt at fixing setup.py (not complete):

from setuptools import setup, find_packages

setup(
    name='audio_visualizer_python',
    version='1.0',
    description='A little GUI tool to render visualization videos of audio files',
    license='MIT',
    url='https://github.com/djfun/audio-visualizer-python',
    packages=find_packages(),
    package_data={
        'src': ['*'],
    },
    install_requires=['Pillow-SIMD', 'numpy'],
    entry_points={
        'gui_scripts': [
            'audio-visualizer-python = avpython:main'
        ]
    }
)
tassaron commented 7 years ago

Did you uninstall Pillow before trying to install Pillow-SIMD? I think that is required. Does installing Pillow-SIMD work using Pip manually? Which dependency can't it find? Pillow-SIMD makes me install olefile on Linux as well, I'm not sure if it can be omitted.

I've been creating my first setuptools-like script in freeze.py. I'm new to this which is why I haven't updated the setup.py file myself. I will try to help though. avpython is a reference to this project, Audio Visualizer Python. The entry point for AVP is now at src/main.py instead of just main.py.

In freeze.py I copy the entire src/ directory using include_files. That keyword needs to be changed here since main.ui is renamed to mainwindow.ui and there are many other files to include.

AsavarTzeth commented 7 years ago

Every build is done in a clean sandbox, so that is not the issue. I think you missunderstood the issue. I have succesfully installed Pillow-SIMD with pip3, no issue at all. The problem is that 'pillow-simd' in setup.py needs to be 'Pillow' or it will fail to find it.

Now moving on. It is good to know that you too are new with this. This week has been my first true exposure to setuptools (and related tools). It is honestly the most confusing system I have ever seen. So I can relate to you being new on this.

I suppose I could continue to try and fix it with your guidance. For example, that last paragraph will surely help me a great deal. I will try my best to read documentation and fix it and get back to you with any questions or updates.

AsavarTzeth commented 7 years ago

So I have been reading https://setuptools.readthedocs.io/en/latest/setuptools.html and https://python-packaging.readthedocs.io/en/latest/minimal.html. I have had a look in other projects and searched the web.

It would seem to be very difficult or impossible even to get this to work with the current directory structure. Your code should be in its own module directory that can be picked up by packages=find_packages('src').

I then found https://github.com/djfun/audio-visualizer-python/pull/20 which seems to be the way to solve this, except it was based of the wrong branch.

dv-anomaly commented 7 years ago

There should not be a problem making it work with the current directory structure. All you need to do is change the current working directory and proceed as normal. Shouldn't need to copy anything. If you look around a lot of other python projects use the same directory structure or similar. I will try to find time to help out with this, but my schedule is pretty full through the end of the month.

AsavarTzeth commented 7 years ago

Can you clarify what you mean with change the current directory? I have to run setup.py install from the root and it won't pick up any packages (even when fixed). The end result is nothing is installed on the system.

I cannot do anything before or after that.

Also, by directory structure I mean setuptools expect your module code to be in its own directory. That would mean adding something like src/avpython.

For example, currently entry_points will not work without this change because it expects avpython to be there.

dv-anomaly commented 7 years ago

Well it looks like one can use package_dir to map the directory.

package_dir={'avpython': 'src'}

Perhaps this can be used as an example: https://github.com/apache/thrift/blob/master/lib/py/setup.py

AsavarTzeth commented 7 years ago

Thank you! I did not know package_dir could be used to map a directory that way. If this is true it is exactly what was missing. I will test it in my build as soon as I am available.

dv-anomaly commented 7 years ago

No problem, if you get it working, be sure to submit a pull request to the feature-newgui branch and reference issue #45. Either @tassaron2 or I will try to get it merged right away.

AsavarTzeth commented 7 years ago

Well I think I got it working or at least got closer. However now if I run the entrypoint script in bin or something like from avpython import main from the python3 command line, I get:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/app/lib/python3.5/site-packages/avpython/main.py", line 5, in <module>
    import core
ImportError: No module named 'core'

It would work if I first change into the module directory, but that sort of defeats the purpose of setup.py.

tassaron commented 7 years ago

What does your setup.py look like now?

AsavarTzeth commented 7 years ago
from setuptools import setup, find_packages

setup(
    name='audio_visualizer_python',
    version='1.0',
    description='A little GUI tool to render visualization videos of audio files',
    license='MIT',
    url='https://github.com/djfun/audio-visualizer-python',
    packages={
        'avpython',
        'avpython.components'
    },
    package_dir={'avpython': 'src'},
    package_data={
        '': ['*'],
    },
    install_requires=['Pillow', 'numpy'],
    entry_points={
        'gui_scripts': [
            'audio-visualizer-python = avpython.main:main'
        ]
    }
)
tassaron commented 7 years ago

Try installing with this branch. For me this setup.py works and installs Pillow-SIMD just fine, but now has problems installing numpy and PyQt5 for some reason. If I install numpy and PyQt5 first then after installation I'm able to launch with avp or python3 -m avpython

AsavarTzeth commented 7 years ago

That works except I get this on running avp:

pkg_resources.DistributionNotFound: The 'Pillow-SIMD' distribution was not found and is required by audio-visualizer-python

I mentioned the reason for this several times already. Because Pillow-SIMD offers drop-in compatibility (I assume that is why) it is installed as Pillow. I am uncertain if there is a way to install it differently. I tried both of these sources:

I install pillow-simd with:

python3 setup.py install --prefix=/app --root=/ --optimize=1

If I apply this patch the GUI launches successfully via avp:

--- setup.py    2017-07-16 09:12:39.276179775 +0200
+++ setup.py    2017-07-16 09:15:48.808924619 +0200
@@ -25,7 +25,7 @@
     package_data={
         'avpython': package_files('src'),
     },
-    install_requires=['olefile', 'Pillow-SIMD', 'PyQt5', 'numpy'],
+    install_requires=['Pillow', 'PyQt5', 'numpy'],
     entry_points={
         'gui_scripts': [
             'avp = avpython.main:main'

Although I realize this is not ideal because if one installs everything automatically like you did this would likely install standard Pillow instead...

By the way you don't need to specify olefile explicitly because it is already an implicit dependency of Pillow/Pillow-SIMD.

AsavarTzeth commented 7 years ago

I finally managed to get it working with Pillow-SIMD set as requirement! (without the patch) I am unsure why but it seems to install differently depending on the combination of where you get the source and the which installation tool is used. That or I messed it up somehow. Anyhow, I am just happy it works.

I would still remove olefile as an explicit, but that is just a detail.

Regarding your own issues with PyQt5 and numpy during installation. I ran into that too.

It would seem this simply is an issue with easy_install, which is used by setup.py when you run python3 setup.py install. Out of the box it simply is not as reliable as using pip. When you want automatic dependency resolution during installation it seems to work better if you run pip3 install .. At first I did not know pip3 supported local directories this way, but clearly it does.

tassaron commented 7 years ago

Hmm. For me putting Pillow-SIMD in install_requires and running it with python3 setup.py install made it complain that it needed olefile, and adding olefile made it install automatically. Strange. We could subclass the installer in setuptools to force it to always use Pip, although that seems like a hacky solution. It appears to be possible.

For now I think changing the install instructions to tell people to run pip install . is a good solution. The setup.py works fine with Pip.

AsavarTzeth commented 7 years ago

Strange indeed, when I tried it some day or two ago, easy_install did try to install it. It failed because there was not network access, but still. As I said, it seems a bit unreliable.

Otherwise, if it helps keep it for now. It should not hurt.

AsavarTzeth commented 7 years ago

I see you merged the setup-fix branch :) Just built and installed from latest commit in feature-newgui branch. All works well, closing this.