Skip to content

Use Python3.7 in CI#3222

Closed
tv3141 wants to merge 4 commits into
SciTools:masterfrom
tv3141:test_python3.7
Closed

Use Python3.7 in CI#3222
tv3141 wants to merge 4 commits into
SciTools:masterfrom
tv3141:test_python3.7

Conversation

@tv3141

@tv3141 tv3141 commented Nov 9, 2018

Copy link
Copy Markdown
Contributor

Changes to .travis.yaml

For testing with Travis, Python is installed using miniconda and only this version of Python is used. Specifying - python: 3.6 in .travis.yml sets the version of Python that comes installed in the Travis container.
Python3.7 caused some problems travis-ci/travis-ci#9069, and requires a workaround using Ubuntu Xenial.

python:
- 2.7

matrix:
  include:
    - python: 3.7
      dist: xenial
      sudo: true

Instead:

  • use minimal travis container

  • specify Python version as env var

  • get Travis to work with Python3.7

Fix failing tests with Python3.7

  • iris.math.IFunc should only accept numpy ufuncs functions accepting numpy arrays

@tv3141

tv3141 commented Nov 9, 2018

Copy link
Copy Markdown
Contributor Author

One failure:

FAIL: test_ifunc_init_fail (iris.tests.test_basic_maths.TestBasicMaths)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/test-environment/lib/python3.7/site-packages/scitools_iris-2.3.0.dev0-py3.7.egg/iris/tests/test_basic_maths.py", line 302, in test_ifunc_init_fail
    lambda cube: cf_units.Unit('1'))
AssertionError: TypeError not raised by IFunc
299:        # should fail because math.sqrt is built-in function, which can not be
300:        # used in inspect.getargspec
301:        self.assertRaises(TypeError, iris.analysis.maths.IFunc, math.sqrt,
302:                          lambda cube: cf_units.Unit('1'))

In Python3.7 math.sqrt seems to be inspectable:

Python 3.7.0 (default, Jun 28 2018, 13:15:42) 
[GCC 7.2.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import inspect, math
>>> inspect.getargspec(math.sqrt)
__main__:1: DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()
ArgSpec(args=['x'], varargs=None, keywords=None, defaults=None)
Python 2.7.15 | packaged by conda-forge | (default, Oct 12 2018, 14:10:50) 
[GCC 4.8.2 20140120 (Red Hat 4.8.2-15)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import inspect, math
>>> inspect.getargspec(math.sqrt)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/user/anaconda3/envs/iris_test2.7/lib/python2.7/inspect.py", line 818, in getargspec
    raise TypeError('{!r} is not a Python function'.format(func))
TypeError: <built-in function sqrt> is not a Python function

@bjlittle bjlittle self-assigned this Nov 11, 2018

@bjlittle bjlittle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Loving that you've got this working for py37! Good job 👍

Is there anyway that we can still use minimal travis containers and yet not have to populate the matrix manually? Particularly if we want to support py27, py36 and py37, this becomes a tad unwieldy and verbose.

Comment thread .travis.yml Outdated
- PYTHON_VERSION=2.7 TEST_TARGET=example
- PYTHON_VERSION=3.7 TEST_TARGET=example
- PYTHON_VERSION=3.7 TEST_TARGET=doctest
# doctests only with Python3, see https://github.com/SciTools/iris/pull/3134

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The downside, for me, is that really we should be covering py27, py36 and py37... At this point, we've lost the convenience of the matrix...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The alternative that I know works with Travis-CI is not better than this. It would require something like:

python:
- 2.7
- 3.6

matrix:
  include:
    - python: 3.7
      dist: xenial
      sudo: true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe some reordering into blocks by python version can make this more appealing:

env:
  matrix:
    - PYTHON_VERSION=2.7 TEST_TARGET=default TEST_MINIMAL=true
    - PYTHON_VERSION=2.7 TEST_TARGET=default
    - PYTHON_VERSION=2.7 TEST_TARGET=example
    # doctests only with Python3, see https://github.com/SciTools/iris/pull/3134
    #- PYTHON_VERSION=2.7 TEST_TARGET=doctest

    - PYTHON_VERSION=3.6 TEST_TARGET=default TEST_MINIMAL=true
    - PYTHON_VERSION=3.6 TEST_TARGET=default
    - PYTHON_VERSION=3.6 TEST_TARGET=example
    - PYTHON_VERSION=3.6 TEST_TARGET=doctest

    - PYTHON_VERSION=3.7 TEST_TARGET=default TEST_MINIMAL=true
    - PYTHON_VERSION=3.7 TEST_TARGET=default
    - PYTHON_VERSION=3.7 TEST_TARGET=example
    - PYTHON_VERSION=3.7 TEST_TARGET=doctest

@tv3141

tv3141 commented Nov 11, 2018

Copy link
Copy Markdown
Contributor Author

I removed the failing test, but I think the intention was that iris.math.IFunc only accepts numpy ufuncs functions accepting numpy arrays, which is not the case anymore.

Python3.7

import sys
sys.version

import numpy as np
import cf_units
import iris
from iris.coords import DimCoord
from iris.cube import Cube

latitude = DimCoord(np.linspace(-90, 90, 4), standard_name='latitude', units='degrees')
longitude = DimCoord(np.linspace(45, 360, 8), standard_name='longitude', units='degrees')
cube = Cube(np.zeros((4, 8), np.float32), dim_coords_and_dims=[(latitude, 0), (longitude, 1)])

# this works
sine_ifunc = iris.analysis.maths.IFunc(np.sin, lambda cube: cf_units.Unit('1'))
sine_cube = sine_ifunc(cube)

# this fails
import math
# should fail in next line
sine_ifunc = iris.analysis.maths.IFunc(math.sin, lambda cube: cf_units.Unit('1'))
sine_cube = sine_ifunc(cube)
'3.7.0 | packaged by conda-forge | (default, Sep 30 2018, 14:56:18) \n[GCC 4.8.2 20140120 (Red Hat 4.8.2-15)]'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/tv/repos/iris/lib/iris/analysis/maths.py", line 1011, in __call__
    in_place=in_place)
  File "/home/tv/repos/iris/lib/iris/analysis/maths.py", line 843, in _math_op_common
    new_cube = cube.copy(data=operation_function(cube.core_data()))
  File "/home/tv/repos/iris/lib/iris/analysis/maths.py", line 989, in wrap_data_func
    return self.data_func(*args, **kwargs_combined)
TypeError: only size-1 arrays can be converted to Python scalars

@tv3141 tv3141 changed the title Use Python3.7 in CI [WIP] Use Python3.7 in CI Nov 11, 2018
@tv3141 tv3141 changed the title [WIP] Use Python3.7 in CI Use Python3.7 in CI Nov 11, 2018
@CAM-Gerlach

Copy link
Copy Markdown

FYI, the sudo: key is obsolete and can be removed, as the containerized infrastructure it refers to was removed from Travis years ago.

@bjlittle

bjlittle commented Jul 9, 2019

Copy link
Copy Markdown
Member

Closed by #3350

Thanks @tv3141 👍

@bjlittle bjlittle closed this Jul 9, 2019
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.

4 participants