mobiusklein / ms_deisotope

A library for deisotoping and charge state deconvolution of complex mass spectra
https://mobiusklein.github.io/ms_deisotope
Apache License 2.0
33 stars 13 forks source link

AttributeError: 'PeakDependenceGraph' object has no attribute 'best_exact_fits' #17

Closed MKoesters closed 4 years ago

MKoesters commented 4 years ago

I'm trying to run this code

from ms_deisotope.deconvolution import deconvolute_peaks
from ms_peak_picker import simple_peak

peaks = [(200, 10000), (203.12312, 3132132)] # example data
peaks = [simple_peak(p[0], p[1], 0.01) for p in peaks]
decon_result = deconvolute_peaks(peaks, *args, **kwargs)

which always worked for me, but started failing with ms_deisotope==0.0.13 under linux with python >3.5, under windows it works fine for me.

Can you tell me if I need to adapt my code or if there is a bug in ms_deisotope?

mobiusklein commented 4 years ago

I recently moved best_exact_fits from Python to Cython. The release passed the test suite which exercises this component on Py3.5-3.7 on Linux. Can you make sure you've completely uninstalled ms_deisotope from your environment, and then do a clean install?

I think I've encountered this type of issue before when I directly execute setup.py instead of pip to install packages from source, since pip always enforces "single version externally managed" mode", but setuptools doesn't automatically.

MKoesters commented 4 years ago

Thanks for your fast answer.

I'm running tests with tox and recreating the environment, so I'm quite confident that I'm not using an older version of ms_deisotope.

Also, this fails in the CI/CD pipeline of pymzML for https://github.com/pymzml/pymzML/pull/210 , see the log here I'm not quite sure if this is an error in our setup and if so, how to fix it

Best, Manuel

mobiusklein commented 4 years ago

Thank you for following up.

There are two issues here. The first is that you're not loading the C extensions for some reason, which is odd. The second is that I didn't indent the conditional declaration of the Python fallback implementation correctly, so half of the methods on it were missing.

I've pushed commit 98cec4573890dfa3363df05b9fb7712382742d2b which fixes the indentation in the Python fallback implementation so all of the methods are properly defined on it.

I suspect you're encountering an error during C extension compilation on those versions of Python, but the error is getting swallowed by whatever you're using to install your dependencies. I'll look around a bit, but I thought my setup.py script would generate loud warning messages if C extensions failed to compile. Is there a verbose option for your environment setup?

MKoesters commented 4 years ago

I'll have a look at the logs and try to run the test CI/CD in verbose mode, I'll come back to you as soon as I found the relevant information.

MKoesters commented 4 years ago

I think I figured out what is going wrong.

When I download ms_deisotope==0.0.13 from pip and run pip install -v . I get the following output:

cd ms_deisotope-0.0.13
pip install -v .
Non-user install because user site-packages disabled
Created temporary directory: /tmp/pip-ephem-wheel-cache-w8adtcxa
Created temporary directory: /tmp/pip-req-tracker-wlr9xe9d
Initialized build tracking at /tmp/pip-req-tracker-wlr9xe9d
Created build tracker: /tmp/pip-req-tracker-wlr9xe9d
Entered build tracker: /tmp/pip-req-tracker-wlr9xe9d
Created temporary directory: /tmp/pip-install-xiw4qdo7
Processing /mnt/Downloads/ms_deisotope-0.0.13
  Created temporary directory: /tmp/pip-req-build-8vzo4lan
  Added file:///mnt/Downloads/ms_deisotope-0.0.13 to build tracker '/tmp/pip-req-tracker-wlr9xe9d'
    Running setup.py (path:/tmp/pip-req-build-8vzo4lan/setup.py) egg_info for package from file:///mnt/Downloads/ms_deisotope-0.0.13
    Running command python setup.py egg_info
    Version is: '0.0.13'
    Installation requires `brainpy`, install with `python -m pip install brain-isotopic-distribution`
    No module named 'brainpy'
    Version is: '0.0.13'
    running egg_info
    creating /tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info
    writing /tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/PKG-INFO
    writing dependency_links to /tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/dependency_links.txt
    writing entry points to /tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/entry_points.txt
    writing requirements to /tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/requires.txt
    writing top-level names to /tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/top_level.txt
    writing manifest file '/tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/SOURCES.txt'
    reading manifest file '/tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/SOURCES.txt'
    reading manifest template 'MANIFEST.in'
    warning: no files found matching '*.h' under directory 'ms_deisotope'
    warning: no previously-included files matching '*.pyd' found under directory 'ms_deisotope'
    warning: no previously-included files matching '*' found under directory 'ms_deisotope/test/test_data'
    writing manifest file '/tmp/pip-req-build-8vzo4lan/pip-egg-info/ms_deisotope.egg-info/SOURCES.txt'
    ***************************************************************************
    WARNING: The C extension could not be compiled, speedups are not enabled.
    Plain-Python build succeeded.
    ***************************************************************************
  Source in /tmp/pip-req-build-8vzo4lan has version 0.0.13, which satisfies requirement ms-deisotope==0.0.13 from file:///mnt/Downloads/ms_deisotope-0.0.13
  Removed ms-deisotope==0.0.13 from file:///mnt/Downloads/ms_deisotope-0.0.13 from build tracker '/tmp/pip-req-tracker-wlr9xe9d'
cd ms_deisotope-0.0.13
pip install -v cython numpy brain-isotopic-distribution .

Doesnt work either, since brainpy is built, but not available to ms_deisotope at that point.

So I guess when installing ms_deisotope from pip, I need to install brainpy manually with a separate pip call before, which is a bit annoying, but rather a problem of pymzMLs or python/pip dependency management, but please correct me if I'm wrong.

mobiusklein commented 4 years ago

Ah, right. Build time dependencies. Thank you for getting the installation log. In order to build the C-extensions, you need Cython, numpy, ms_peak_picker, and brain-isotopic-distribution compiled and installed prior to executing pip install ms_deisotope, and because pip squelches any messages that do not actually kill the build, nobody would know this is happening.

Right now, what you need to do is the following to make sure all build-time dependencies are in place in-order:

pip install Cython numpy
pip install brain-isotopic-distribution
pip install  ms_peak_picker
pip install ms_deisotope

This sounds like it needs to be tackled both as a documentation issue and a build system issue.

I'll cut a release tonight that explicitly lists these instructions, patches the pure Python implementation on PyPI, and start looking into the recent build system tooling that is supposed to make build time dependencies reasonable to use.

mobiusklein commented 4 years ago

Okay. A 0.0.14 release for ms_deisotope is live on PyPI which has all of the above changes and PEP 517/518 build isolation baked in, as does ms_peak_picker and brainpy, so they will ensure that all their build time dependencies are available first.

Testing with a clean environment seemed to work for me, but I am still uncertain I set this up correctly. Please let me know if they work for you.

MKoesters commented 4 years ago

I tested everything locally and using CI/CD, your fixes resolved the problem Thank you really much for resolving this issue that fast!