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

[BUG] pyradiomics breaking CI when installed as a dependency #642

Closed
mluerig opened this issue Sep 29, 2020 · 24 comments
Closed

[BUG] pyradiomics breaking CI when installed as a dependency #642

mluerig opened this issue Sep 29, 2020 · 24 comments
Labels

Comments

@mluerig
Copy link

mluerig commented Sep 29, 2020

Describe the bug
As of recently I am getting an error when installing pyradiomics as a dependency of my package during CI

CI log file
$ source ~/virtualenv/python3.7/bin/activate
$ python --version
Python 3.7.1
$ pip --version
pip 20.1.1 from /home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/pip (python 3.7)
install.1
2.45s$ python -m pip install -U pip
install.2
1.42s$ pip install tox
33.59s$ tox
GLOB sdist-make: /home/travis/build/mluerig/phenopype/setup.py
py37 create: /home/travis/build/mluerig/phenopype/.tox/py37
py37 installdeps: .[test]
py37 inst: /home/travis/build/mluerig/phenopype/.tox/.tmp/package/1/phenopype-1.0.7.zip
py37 installed: attrs==20.2.0,certifi==2020.6.20,chardet==3.0.4,coverage==5.3,coveralls==2.1.2,docopt==0.6.2,EasyProcess==0.3,idna==2.10,importlib-metadata==2.0.0,iniconfig==1.0.1,mock==4.0.2,numpy==1.18.5,opencv-contrib-python==3.4.9.33,packaging==20.4,pandas==1.1.2,pathtools==0.1.2,phenopype==1.0.7,Pillow==7.2.0,pluggy==0.13.1,py==1.9.0,pykwalify==1.7.0,pyparsing==2.4.7,pyradiomics==3.0,pytest==6.1.0,pytest-cov==2.10.1,pytest-xvfb==2.0.0,python-dateutil==2.8.1,pytz==2020.1,PyVirtualDisplay==1.3.2,PyWavelets==1.0.0,PyYAML==5.3.1,requests==2.24.0,ruamel.yaml==0.16.12,ruamel.yaml.clib==0.2.2,SimpleITK==2.0.0,six==1.15.0,toml==0.10.1,tqdm==4.50.0,urllib3==1.25.10,watchdog==0.10.3,zipp==3.2.0
py37 run-test-pre: PYTHONHASHSEED='2973730562'
py37 run-test: commands[0] | pytest -s --cov=phenopype --cov-report=html
ImportError while loading conftest '/home/travis/build/mluerig/phenopype/tests/conftest.py'.
tests/conftest.py:5: in
import phenopype as pp
phenopype/init.py:3: in
from .main import project, pype
phenopype/main.py:23: in
from phenopype.core import preprocessing, segmentation, measurement, export, visualization
phenopype/core/measurement.py:9: in
from radiomics import featureextractor
.tox/py37/lib/python3.7/site-packages/radiomics/init.py:15: in
from . import imageoperations
.tox/py37/lib/python3.7/site-packages/radiomics/imageoperations.py:6: in
import pywt
.tox/py37/lib/python3.7/site-packages/pywt/init.py:16: in
from ._extensions._pywt import *
pywt/_extensions/_pywt.pyx:1: in init pywt._extensions._pywt
???
pywt/_extensions/_dwt.pyx:7: in init pywt._extensions._dwt
???
E ImportWarning: can't resolve package from spec or package, falling back on name and path
ERROR: InvocationError for command /home/travis/build/mluerig/phenopype/.tox/py37/bin/pytest -s --cov=phenopype --cov-report=html (exited with code 4)
___________________________________ summary ____________________________________
ERROR: py37: commands failed
The command "tox" exited with 1.

To Reproduce
https://ci.appveyor.com/project/mluerig/phenopype
https://travis-ci.org/github/mluerig/phenopype

Version (please complete the following information):

  • OS: windows, linux
  • Python version: 3.7
  • PyRadiomics version: 3.0

Additional context
looks like pywt is breaking here - any ideas? thanks!

@mluerig mluerig added the bug label Sep 29, 2020
@mluerig mluerig changed the title [BUG] [BUG] pyradiomics breaking CI when installed as a dependency Sep 29, 2020
@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

As far as I can see from your log, It's failing to find the PyWavelets extensions (pywt), can you try installing this first? It's listed in the dependencies of PyRadiomics, so in theory, it should get installed when you try to install PyRadiomics...

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

I can try, but I'm unsure where should I inject this - setup.py, tox.ini, or travis/appveyor.yaml?

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

Just before pip install tox in travis/appveyor.yml

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

that also doesn't work, unfortunately

what I also tried was to remove the .tox folder on my local repo, so it would be rebuilt (because it still ran locally). now with the .tox folder rebuilt, it also fails to run on my local machine

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

Hmm, upon reviewing your code, apparently pywt was installed, but it seems it can't load the C extensions. Can you see if you can install and import pywavelets on your local machine?

It doesn't appear to be a problem in PyRadiomics per se, but rather something in the PyWavelets package PyRadiomics depends on for the wavelet filter...

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

yeah that works fine (pip install PyWavelets / import pywt)

not sure how to proceed from here...ping the PyWavelets folks?

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

Hmm maybe. I do remember we had some issues with the automated build stuff from PyWavelets before. Can you try to get tox to install pywavelets (e.g0.5.2)? Due to that issue, the PyRadiomics requirements file specifies pywavelet > 0.4.0, <= 1.0.0.

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

I tried with PyWavelets==0.5.2 (via setup.py test_deps), but that didn't work either - same error

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

Hmm, maybe ask the PyWavelets guys then, as far as I can see, the compiled code is weird in a way that python seems to be able to load it, but is then unable to extract the necessary information (such as module and function names) from it.

Can you link the PyWavelets issue you find/create to this issue? It would help other user running into this issue via a PyRadiomics build.

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

quick question before I raise the issue there: why does pyradiomics have pywavelets==1.0.0 as a requirement? PyWavelets/pywt#467 indicates that pywavelets>1.0.2 fixes a similar issue...

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

That was due to this issue #50 in the SlicerRadiomics GUI for Slicer. After that version they made some changes in the build system causing it to fail in the build workflow for slicer (which at that point required to build all C extensions from source due to different versions of compilers).

I'll check to see if this can be removed now, as Slicer has since upgrade to use of Python3 with the same version compilers.

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

Are you able to test your workflow where you update the requirements.txt in the PyRadiomics repository? If you remove the check <= 1.0.0 on the PyWavelets line, it should install the latest version.

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

not sure I understand - where would I remove the check - somewhere in .tox?

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

oh, you mean by downloading the current repo of pyradiomics, modifying it, and then supplying it as a requirement into the workflow of my package, e.g. via a pip dev install?

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

Ok other fix. I removed the upper limit from the PyWavelet requirement in PyRadiomics' master (to also test if it's still necessary). Can you re-try your workflow on your local machine, installing PyRadiomics from source?

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

At the moment, tox installs PyRadiomics 3.0, not the latest master. Can you update tox or your dependencies file to install the latest PyRadiomics version? If not, you'd have to wait until the next PyRadiomics release (I hope to do that sometime this week though).

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 29, 2020

oh, you mean by downloading the current repo of pyradiomics, modifying it, and then supplying it as a requirement into the workflow of my package, e.g. via a pip dev install?

Yes, though I don't know if just injecting it into your workflow will work. It looks like tox works inside it's own virtual environment.
Anyway, installing directly from a git repo can be done using pip:

pip install git+https://github.com/Radiomics/pyradiomics.git

@mluerig
Copy link
Author

mluerig commented Sep 29, 2020

I tried with 'pyradiomics @ git+https://github.com/Radiomics/pyradiomics#f41002bd7c8cce8d75cb5a4854adcdc4e7f30e27' in my setup.py. this seemed to work for the injection, but now I get this error - this is indicates that numpy installation failed during pyradiomics import? weird

edit: whoops forgot the msg

(pp37) E:\git_repos\phenopype>cmd /k
(pp37) E:\git_repos\phenopype>tox
GLOB sdist-make: E:\git_repos\phenopype\setup.py
py37 create: E:\git_repos\phenopype\.tox\py37
py37 installdeps: .[test]
ERROR: invocation failed (exit code 1), logfile: E:\git_repos\phenopype\.tox\py37\log\py37-1.log
====================================================== log start ======================================================
Processing e:\git_repos\phenopype
Collecting pyradiomics@ git+https://github.com/Radiomics/pyradiomics#f41002bd7c8cce8d75cb5a4854adcdc4e7f30e27
  Cloning https://github.com/Radiomics/pyradiomics to c:\users\mluerig\appdata\local\temp\pip-install-v9c6r9no\pyradiomics
    ERROR: Command errored out with exit status 1:
     command: 'E:\git_repos\phenopype\.tox\py37\Scripts\python.EXE' -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\mluerig\\AppData\\Local\\Temp\\pip-install-v9c6r9no\\pyradiomics\\setup.py'"'"'; __file__='"'"'C:\\Users\\mluerig\\AppData\\Local\\Temp\\pip-install-v9c6r9no\\pyradiomics\\setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base 'C:\Users\mluerig\AppData\Local\Temp\pip-pip-egg-info-ckyuwu_w'
         cwd: C:\Users\mluerig\AppData\Local\Temp\pip-install-v9c6r9no\pyradiomics\
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\Users\mluerig\AppData\Local\Temp\pip-install-v9c6r9no\pyradiomics\setup.py", line 7, in <module>
        import numpy
    ModuleNotFoundError: No module named 'numpy'
    ----------------------------------------
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
WARNING: You are using pip version 20.2.2; however, version 20.2.3 is available.
You should consider upgrading via the 'E:\git_repos\phenopype\.tox\py37\Scripts\python.EXE -m pip install --upgrade pip' command.

======================================================= log end =======================================================
ERROR: could not install deps [.[test]]; v = InvocationError("'E:\\git_repos\\phenopype\\.tox\\py37\\Scripts\\python.EXE' -m pip install '.[test]'", 1)
_______________________________________________________ summary _______________________________________________________
ERROR:   py37: could not install deps [.[test]]; v = InvocationError("'E:\\git_repos\\phenopype\\.tox\\py37\\Scripts\\python.EXE' -m pip install '.[test]'", 1)

@JoostJM
Copy link
Collaborator

JoostJM commented Sep 30, 2020

Ah no, that is actually a different error ;-)
PyRadiomics requires numpy to be installed before running setup.py (this is because it needs the numpy headers to compile the extension code). Installing from wheel does not give such an error, as setup.py is not run for a wheel install.

You can fix this by adding a pip install numpy prior to installing PyRadiomics.

@JoostJM
Copy link
Collaborator

JoostJM commented Oct 12, 2020

@mluerig, were you able to resolve this issue?

@mluerig
Copy link
Author

mluerig commented Oct 12, 2020

give me a few more days and I´ll get back to you

@mluerig
Copy link
Author

mluerig commented Oct 19, 2020

I saw you released 3.0.1 and I changed my setup.py back to what it was before. now I get this:

(pp37) E:\git_repos\phenopype>tox
GLOB sdist-make: E:\git_repos\phenopype\setup.py
py37 create: E:\git_repos\phenopype\.tox\py37
py37 installdeps: .[test]
py37 inst: E:\git_repos\phenopype\.tox\.tmp\package\1\phenopype-1.0.8.zip
py37 installed: atomicwrites==1.4.0,attrs==20.2.0,certifi==2020.6.20,chardet==3.0.4,colorama==0.4.4,coverage==5.3,coveralls==2.1.2,docopt==0.6.2,EasyProcess==0.3,idna==2.10,importlib-metadata==2.0.0,iniconfig==1.1.1,mock==4.0.2,numpy==1.18.5,opencv-contrib-python==3.4.9.33,packaging==20.4,pandas==1.1.3,pathtools==0.1.2,phenopype @ file:///E:/git_repos/phenopype/.tox/.tmp/package/1/phenopype-1.0.8.zip,Pillow==8.0.0,pluggy==0.13.1,py==1.9.0,pykwalify==1.7.0,pyparsing==2.4.7,pyradiomics==3.0.1,pytest==6.1.1,pytest-cov==2.10.1,pytest-xvfb==2.0.0,python-dateutil==2.8.1,pytz==2020.1,PyVirtualDisplay==1.3.2,PyWavelets==1.1.1,PyYAML==5.3.1,requests==2.24.0,ruamel.yaml==0.16.12,ruamel.yaml.clib==0.2.2,SimpleITK==2.0.0,six==1.15.0,toml==0.10.1,tqdm==4.50.2,urllib3==1.25.10,watchdog==0.10.3,zipp==3.3.1
py37 run-test-pre: PYTHONHASHSEED='448'
py37 run-test: commands[0] | pytest -s --cov=phenopype --cov-report=html
ImportError while loading conftest 'E:\git_repos\phenopype\tests\conftest.py'.
tests\conftest.py:5: in <module>
    import phenopype as pp
phenopype\__init__.py:3: in <module>
    from .main import project, pype
phenopype\main.py:23: in <module>
    from phenopype.core import preprocessing, segmentation, measurement, export, visualization
phenopype\core\measurement.py:9: in <module>
    from radiomics import featureextractor
.tox\py37\lib\site-packages\radiomics\__init__.py:302: in <module>
    getFeatureClasses()
.tox\py37\lib\site-packages\radiomics\__init__.py:87: in getFeatureClasses
    __import__('radiomics.' + mod)
.tox\py37\lib\site-packages\radiomics\featureextractor.py:11: in <module>
    import pykwalify.core
.tox\py37\lib\site-packages\pykwalify\core.py:7: in <module>
    import imp
c:\anaconda3\envs\pp37\lib\imp.py:33: in <module>
    DeprecationWarning, stacklevel=2)
E   DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
ERROR: InvocationError for command 'E:\git_repos\phenopype\.tox\py37\Scripts\pytest.EXE' -s --cov=phenopype --cov-report=html (exited with code 4)
_______________________________________________________ summary _______________________________________________________
ERROR:   py37: commands failed

@JoostJM
Copy link
Collaborator

JoostJM commented Oct 20, 2020

We're getting there... this tells me the installation at least worked, as it's trying to run some tests.
The only "error" I see is the deprecation warning. This is not caused by PyRadiomics, but rather pykwalify, which is used to validate the parameter files.

2 thing you can do:

  • contact the developers from pykwalify to be aware of this deprecation.
  • suppress the warning using pythons warnings library. I think it was something like warnings.ignore(DeprecationWarning)

@mluerig
Copy link
Author

mluerig commented Oct 20, 2020

bingo. I fixed it with adding addopts = -p no:warnings to my pytest.ini

will notify the developers of pykwalify

cheers

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

No branches or pull requests

2 participants