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

Compatibility fixes for stable branch #3455

Merged
merged 11 commits into from Jan 16, 2018

Conversation

chris-allan
Copy link
Contributor

@chris-allan chris-allan commented Jan 15, 2018

A couple of compatibility fixes targeting the stable branch which backport changes from master for those of us who are either still using 2.2.x or need to compare 2.2.x outputs with 3.x.

Without 65aa2e0, which is a backport of changes from #2891, an installation from source today when using pipelines which contain the MeasureObjectSizeShape module and have Zernike calculation turned off will throw the following exception:

Error detected during run of module MeasureObjectSizeShape
Traceback (most recent call last):
  File "/home/cp/CellProfiler/cellprofiler/pipeline.py", line 1819, in run_with_yield
    self.run_module(module, workspace)
  File "/home/cp/CellProfiler/cellprofiler/pipeline.py", line 2067, in run_module
    module.run(workspace)
  File "/home/cp/CellProfiler/cellprofiler/modules/measureobjectsizeshape.py", line 291, in run
    self.run_on_objects(object_group.name.value, workspace)
  File "/home/cp/CellProfiler/cellprofiler/modules/measureobjectsizeshape.py", line 369, in run_on_objects
    zf_l = cpmz.zernike(zernike_numbers, labels, indices)
  File "/home/cp/venv/local/lib/python2.7/site-packages/centrosome/zernike.py", line 186, in zernike
    zf = construct_zernike_polynomials(x, y, zernike_indexes, mask)
  File "/home/cp/venv/local/lib/python2.7/site-packages/centrosome/zernike.py", line 57, in construct_zernike_polynomials
    lut = construct_zernike_lookuptable(zernike_indexes) # precompute poly. coeffs.
  File "/home/cp/venv/local/lib/python2.7/site-packages/centrosome/zernike.py", line 16, in construct_zernike_lookuptable
    n_max = np.max(zernike_indexes[:,0])
TypeError: list indices must be integers, not tuple

Example pipeline used:

An alternative is to pin centrosome to 1.0.5. This was the version that 2.2.0 was released with.

CellProfiler 2.2.x is also incompatible with numpy >= 1.12.0 so pinning this version makes sense to avoid users shooting themselves in the foot. This was reflected in #2588 and relaxed in #2596 with the addition of support for 1.12.0+. An alternative would be to backport the changes from #2596 while being quite careful about the corresponding centrosome dependency.

/cc @emilroz, @stick

Backport of 75e7b2c which was merged as
part of 1ee421f in CellProfiler#2891.

During development of CellProfiler/centrosome#74 and
CellProfiler/centrosome#75, an API incompatibility was introduced where
`centrosome.zernike.zernike()` and by extension
`centrosome.zernike.construct_zernike_lookuptable()` no longer accept an
empty list as input for `zernike_indexes`.  This makes
`centrosome>=1.0.8` incompatible with `CellProfiler<=3.0.0` and source
installs of such versions not work with pipelines where Zernike
calculation is turned off.
@chris-allan chris-allan force-pushed the stable-compat branch 2 times, most recently from 59e134e to 7b567f7 Compare January 15, 2018 16:21
@chris-allan
Copy link
Contributor Author

Now that Travis is passing some notes follow related to the above commits for anyone who wants to understand rationale or comes across any of these problems during deployment:

  1. Travis is running based on Ubuntu 14.04. Having a large number of Python modules installed via their Debian packages creates a great deal of potential incompatibilities with modules installed via pip later. Most of these have been cleaned up.
  2. numpy has to be installed before the CellProfiler requirements are processed so that javabridge can be installed correctly and target the correct numpy version.
  3. Travis caches are per repository, per branch. However, stable seems to have no cache so the default master branch cache is used where a large number of the wheels present are built against numpy >= 0.12.0. These wheels are ABI incompatible with numpy 0.11.3. It is possible that Travis will update the cache once this or similar PRs are merged. For the time being not relying on the $HOME/.cache/pip cache has a marginal increase in build time which is relatively insignificant compared to the test run time.
  4. Several tests rely on master branch locations of various files. These have been updated to target the 2.2.0 tagged version.
  5. The tests rely on tags being present so that git describe --tags can execute cleanly. These need to be fetched explicitly with how Travis clones and fetches the PR references in Git. The most recent describable tag reachable on stable is v2.2.1 so the test cases have had to be modified to handle that.
  6. http://cellprofiler.org/CPupdate.html no longer exists so I have thus skipped these tests. I'm not sure if this is all intended. CP 3.0.0 has had upgrade checks stripped out in remove version check #1878 and upgrade checks are currently broken in the released 2.2.0.
  7. The image set cache is broken with versions of h5py >= 2.7.0. It is implied in Remove caching from Image, Object, and Workspace #2229 that while there are tests targetting it, the class isn't itself actually used anywhere. Lacking more information I have pinned the h5py version at < 2.7.0.

Instances of [3] related to javabridge will manifest themselves as::

...
RuntimeError: module compiled against API version 0xb but this version of numpy is 0xa
...

and:

...
Traceback (most recent call last):
  File "setup.py", line 461, in <module>
    version="2.2.0"
  File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/setuptools/__init__.py", line 129, in setup
    return distutils.core.setup(**attrs)
  File "/usr/lib/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/usr/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "setup.py", line 153, in run
    import cellprofiler.utilities.cpjvm
  File "/home/travis/build/CellProfiler/CellProfiler/cellprofiler/utilities/cpjvm.py", line 3, in <module>
    import javabridge
  File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/javabridge/__init__.py", line 38, in <module>
    from .jutil import start_vm, kill_vm, vm, activate_awt, deactivate_awt
  File "/home/travis/virtualenv/python2.7_with_system_site_packages/local/lib/python2.7/site-packages/javabridge/jutil.py", line 157, in <module>
    import javabridge._javabridge as _javabridge
AttributeError: 'module' object has no attribute '_javabridge'
...

Instances of [7] will manifest themselves as:

...
ERROR: test_03_01_errors (__main__.TestImageSetCache)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "cellprofiler/tests/test_measurements.py", line 1417, in test_03_01_errors
    image_set_data = cache.get_image_set_data(i)
  File "/home/cp/CellProfiler/cellprofiler/measurements.py", line 2210, in get_image_set_data
    errors = self.get_errors(idx)
  File "/home/cp/CellProfiler/cellprofiler/measurements.py", line 2206, in get_errors
    for i in errors]
  File "/home/cp/CellProfiler/cellprofiler/utilities/hdf5_dict.py", line 2005, in __getitem__
    begin, end = self.index[idx, :]
ValueError: need more than 1 value to unpack
...

Copy link
Contributor

@mcquin mcquin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, @chris-allan! Thanks for providing context along with your changes.

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

Successfully merging this pull request may close these issues.

None yet

2 participants