Make most keyword parameters invalid as positional arguments

.github/workflows/ci.yml

@@ -22,26 +22,26 @@ jobs:
             matrix:
                 # First all python versions in basic linux
                 os: [ ubuntu-latest ]
-                py: [ 2.7, 3.5, 3.6, 3.7, 3.8, 3.9, pypy3 ]
+                py: [ 3.6, 3.7, 3.8, 3.9, pypy3 ]
                 CC: [ gcc ]
                 CXX: [ g++ ]
 
                 # Add some other particular combinations to test
                 include:
                     # A couple in MacOS
                     - os: macos-latest
-                      py: 2.7
+                      py: 3.7
                       CC: cc
                       CXX: c++
 
                     - os: macos-latest
-                      py: 3.7
+                      py: 3.9
                       CC: cc
                       CXX: c++
 
                     # Check one with clang compiler
                     - os: ubuntu-latest
-                      py: 3.7
+                      py: 3.8
                       CC: clang
                       CXX: clang++
 

CHANGELOG.rst

@@ -6,9 +6,16 @@ See the listing below for the complete list of new features and changes.
 <https://github.com/rmjarvis/TreeCorr/issues?q=milestone%3A%22Version+4.3%22+is%3Aclosed>`_
 whose issue numbers are listed below for the relevant items.
 
+Starting with this version, TreeCorr no longer supports Python 2.7.
+We currently support python versions 3.6, 3.7, 3,8, 3.9.
+
+
 API Changes
 -----------
 
+- Many function parameters are now keyword-only.  The old syntax allowing these parameters
+  to be positional still works, but is deprecated. (#129)
+
 
 Performance improvements
 ------------------------

setup.py

@@ -89,8 +89,6 @@ def get_compiler_type(compiler, check_unknown=True, output=False):
         print('compiler version information: ')
         for line in lines:
             print(line.decode().strip())
-    # Python3 needs this decode bit.
-    # Python2.7 doesn't need it, but it works fine.
     line = lines[0].decode(encoding='UTF-8')
     if line.startswith('Configured'):
         line = lines[1].decode(encoding='UTF-8')

tests/test_catalog.py

@@ -23,6 +23,7 @@
 from numpy import pi
 import fitsio
 import treecorr
+from unittest import mock
 
 from test_helper import get_from_wiki, CaptureLog, assert_raises, do_pickle, timer, assert_warns
 
@@ -900,9 +901,6 @@ def test_ext():
                          k_col='k', g1_col='g1', g2_col='g2',
                          ext=1)
 
-    if sys.version_info < (3,): return  # mock only available on python 3
-    from unittest import mock
-
     # test that the case where we can't slice works
     # by pretending that we are using an old fitsio version,
     # temporarily.
@@ -1577,6 +1575,14 @@ def test_list():
         np.testing.assert_almost_equal(cats[k].x, x_list[k])
         np.testing.assert_almost_equal(cats[k].y, y_list[k])
 
+    # Providing positional args past list_key is now deprecated.
+    with assert_warns(FutureWarning):
+        cats = treecorr.read_catalogs(config, 'file_name', 'file_list', 0, None, None)
+    np.testing.assert_equal(len(cats), ncats)
+    for k in range(ncats):
+        np.testing.assert_almost_equal(cats[k].x, x_list[k])
+        np.testing.assert_almost_equal(cats[k].y, y_list[k])
+
 @timer
 def test_write():
     # Test that writing a Catalog to a file and then reading it back in works correctly
@@ -1736,12 +1742,12 @@ def test_field():
 
     t0 = time.time()
     nfield1 = cat1.getNField()
-    nfield2 = cat2.getNField(0.01, 1)
-    nfield3 = cat3.getNField(1,300, logger=logger)
+    nfield2 = cat2.getNField(min_size=0.01, max_size=1)
+    nfield3 = cat3.getNField(min_size=1, max_size=300, logger=logger)
     t1 = time.time()
     nfield1b = cat1.getNField()
-    nfield2b = cat2.getNField(0.01, 1)
-    nfield3b = cat3.getNField(1,300, logger=logger)
+    nfield2b = cat2.getNField(min_size=0.01, max_size=1)
+    nfield3b = cat3.getNField(min_size=1, max_size=300, logger=logger)
     t2 = time.time()
     assert cat1.nfields.count == 1
     assert cat2.nfields.count == 1
@@ -1756,14 +1762,30 @@ def test_field():
     print('nfield: ',t1-t0,t2-t1)
     assert t2-t1 < t1-t0
 
+    # Check warning if not using kwargs for now-kwarg-only params
+    with assert_warns(FutureWarning):
+        nfield2c = cat2.getNField(0.01, 1)
+    with assert_warns(FutureWarning):
+        nfield2d = cat2.getNField(0.01, max_size=1)
+    with assert_warns(FutureWarning):
+        nfield2e = cat2.getNField(0.01, 1, None, False, None, 10, None, None)
+    with assert_raises(TypeError):
+        # One too many, so this is still an error.
+        nfield2e = cat2.getNField(0.01, 1, None, False, None, 10, None, None, 77)
+    for k in nfield2.__dict__:
+        if k != 'data':
+            assert nfield2c.__dict__[k] == nfield2.__dict__[k]
+            assert nfield2d.__dict__[k] == nfield2.__dict__[k]
+            assert nfield2e.__dict__[k] == nfield2.__dict__[k]
+
     t0 = time.time()
     gfield1 = cat1.getGField()
-    gfield2 = cat2.getGField(0.01, 1)
-    gfield3 = cat3.getGField(1,300, logger=logger)
+    gfield2 = cat2.getGField(min_size=0.01, max_size=1)
+    gfield3 = cat3.getGField(min_size=1, max_size=300, logger=logger)
     t1 = time.time()
     gfield1b = cat1.getGField()
-    gfield2b = cat2.getGField(0.01, 1)
-    gfield3b = cat3.getGField(1,300, logger=logger)
+    gfield2b = cat2.getGField(min_size=0.01, max_size=1)
+    gfield3b = cat3.getGField(min_size=1, max_size=300, logger=logger)
     t2 = time.time()
     assert_raises(TypeError, cat4.getGField)
     assert cat1.gfields.count == 1
@@ -1780,12 +1802,12 @@ def test_field():
 
     t0 = time.time()
     kfield1 = cat1.getKField()
-    kfield2 = cat2.getKField(0.01, 1)
-    kfield3 = cat3.getKField(1,300, logger=logger)
+    kfield2 = cat2.getKField(min_size=0.01, max_size=1)
+    kfield3 = cat3.getKField(min_size=1, max_size=300, logger=logger)
     t1 = time.time()
     kfield1b = cat1.getKField()
-    kfield2b = cat2.getKField(0.01, 1)
-    kfield3b = cat3.getKField(1,300, logger=logger)
+    kfield2b = cat2.getKField(min_size=0.01, max_size=1)
+    kfield3b = cat3.getKField(min_size=1, max_size=300, logger=logger)
     t2 = time.time()
     assert_raises(TypeError, cat4.getKField)
     assert cat1.kfields.count == 1
@@ -1883,12 +1905,12 @@ def test_field():
 
     t0 = time.time()
     nfield1 = cat1.getNField()
-    nfield2 = cat1.getNField(0.01, 1)
-    nfield3 = cat1.getNField(1,300, logger=logger)
+    nfield2 = cat1.getNField(min_size=0.01, max_size=1)
+    nfield3 = cat1.getNField(min_size=1, max_size=300, logger=logger)
     t1 = time.time()
     nfield1b = cat1.getNField()
-    nfield2b = cat1.getNField(0.01, 1)
-    nfield3b = cat1.getNField(1,300, logger=logger)
+    nfield2b = cat1.getNField(min_size=0.01, max_size=1)
+    nfield3b = cat1.getNField(min_size=1, max_size=300, logger=logger)
     t2 = time.time()
     assert cat1.nfields.count == 3
     print('after resize(3) nfield: ',t1-t0,t2-t1)
@@ -1921,12 +1943,12 @@ def test_field():
     assert cat1.nfields.size == 0
     t0 = time.time()
     nfield1 = cat1.getNField()
-    nfield2 = cat1.getNField(0.01, 1)
-    nfield3 = cat1.getNField(1,300, logger=logger)
+    nfield2 = cat1.getNField(min_size=0.01, max_size=1)
+    nfield3 = cat1.getNField(min_size=1, max_size=300, logger=logger)
     t1 = time.time()
     nfield1b = cat1.getNField()
-    nfield2b = cat1.getNField(0.01, 1)
-    nfield3b = cat1.getNField(1,300, logger=logger)
+    nfield2b = cat1.getNField(min_size=0.01, max_size=1)
+    nfield3b = cat1.getNField(min_size=1, max_size=300, logger=logger)
     t2 = time.time()
     # This time, not much time difference.
     print('after resize(0) nfield: ',t1-t0,t2-t1)

tests/test_config.py

@@ -18,6 +18,7 @@
 import logging
 import fitsio
 import numpy as np
+from unittest import mock
 
 from test_helper import CaptureLog, assert_raises, timer, assert_warns
 
@@ -337,8 +338,6 @@ def test_check():
 
     # corr2 has a list of standard aliases
     # It is currently empty, but let's mock it up to test the functionality.
-    if sys.version_info < (3,): return  # mock only available on python 3
-    from unittest import mock
     with mock.patch('treecorr.corr2_aliases', {'n2_file_name' : 'nn_file_name'}):
         with assert_warns(FutureWarning):
             config2 = treecorr.config.check_config(config1.copy(), valid_params,
@@ -496,8 +495,6 @@ def test_omp():
     # It's hard to tell what happens in the next step, since we can't control what
     # treecorr._lib.SetOMPThreads does.  It depends on whether OpenMP is enabled and
     # how many cores are available.  So let's mock it up.
-    if sys.version_info < (3,): return  # mock only available on python 3
-    from unittest import mock
     with mock.patch('treecorr.util._lib') as _lib:
         # First mock with OpenMP enables and able to use lots of threads
         _lib.SetOMPThreads = lambda x: x
@@ -564,9 +561,9 @@ def test_gen_read_write():
 
     with assert_raises(ValueError):
         treecorr.util.gen_read(file_name, file_type='Invalid')
-    with assert_raises((OSError, IOError)):  # IOError on py2.7
+    with assert_raises(OSError):
         treecorr.util.gen_read(file_name, file_type='ASCII')
-    with assert_raises((OSError, IOError)):
+    with assert_raises(OSError):
         treecorr.util.gen_read(file_name, file_type='FITS')
 
     # Now some working I/O
@@ -665,8 +662,6 @@ def test_gen_read_write():
         assert 'assumed to be HDF' in cl.output
 
     # Check that errors are reasonable if fitsio not installed.
-    if sys.version_info < (3,): return  # mock only available on python 3
-    from unittest import mock
     with mock.patch.dict(sys.modules, {'fitsio':None}):
         with assert_raises(ImportError):
             treecorr.util.gen_write(file_name2, ['a', 'b'], [a,b])
@@ -735,9 +730,9 @@ def test_gen_multi_read_write():
 
     with assert_raises(ValueError):
         treecorr.util.gen_multi_read(file_name, names, file_type='Invalid')
-    with assert_raises((OSError, IOError)):
+    with assert_raises(OSError):
         treecorr.util.gen_multi_read(file_name, names, file_type='ASCII')
-    with assert_raises((OSError, IOError)):
+    with assert_raises(OSError):
         treecorr.util.gen_multi_read(file_name, names, file_type='FITS')
 
 
@@ -833,15 +828,13 @@ def test_gen_multi_read_write():
     alt_names = ['k1','k2','k3']
     with assert_raises(OSError):
         treecorr.util.gen_multi_read(file_name3, alt_names, logger=cl.logger)
-    with assert_raises((OSError, IOError)):
+    with assert_raises(OSError):
         treecorr.util.gen_multi_read(file_name4, alt_names, logger=cl.logger)
     if h5py:
         with assert_raises(OSError):
             treecorr.util.gen_multi_read(file_name5, alt_names, logger=cl.logger)
 
     # Check that errors are reasonable if fitsio not installed.
-    if sys.version_info < (3,): return  # mock only available on python 3
-    from unittest import mock
     with mock.patch.dict(sys.modules, {'fitsio':None}):
         with assert_raises(ImportError):
             treecorr.util.gen_multi_write(file_name2, col_names, names, data)

tests/test_gg.py

@@ -667,7 +667,7 @@ def test_mapsq():
     # (We provide the range where the results worked out well above.)
     R = gg.rnom[16::2]
     print('R = ',R)
-    mapsq, mapsq_im, mxsq, mxsq_im, varmapsq = gg.calculateMapSq(R)
+    mapsq, mapsq_im, mxsq, mxsq_im, varmapsq = gg.calculateMapSq(R=R)
     true_mapsq = true_mapsq[16::2]
     print('mapsq = ',mapsq)
     print('true_mapsq = ',true_mapsq)
@@ -679,6 +679,15 @@ def test_mapsq():
     print('max = ',max(abs(mxsq)))
     np.testing.assert_allclose(mxsq, 0., atol=3.e-8)
 
+    # Giving R as a positional argument is currently still allowed, but deprecated.
+    with assert_warns(FutureWarning):
+        mapsq_2, mapsq_im_2, mxsq_2, mxsq_im_2, varmapsq_2 = gg.calculateMapSq(R)
+    np.testing.assert_array_equal(mapsq_2, mapsq)
+    np.testing.assert_array_equal(mapsq_im_2, mapsq_im)
+    np.testing.assert_array_equal(mxsq_2, mxsq)
+    np.testing.assert_array_equal(mxsq_im_2, mxsq_im)
+    np.testing.assert_array_equal(varmapsq_2, varmapsq)
+
     mapsq_file = 'output/gg_m2b.txt'
     gg.writeMapSq(mapsq_file, R=R, precision=16)
     data = np.genfromtxt(mapsq_file, names=True)
@@ -750,7 +759,7 @@ def test_mapsq():
     # (We provide the range where the results worked out well above.)
     R = gg.rnom[6:40:4]
     print('R = ',R)
-    gamsq, vargamsq, gamsq_e, gamsq_b, vargamsq_eb = gg.calculateGamSq(R, eb=True)
+    gamsq, vargamsq, gamsq_e, gamsq_b, vargamsq_eb = gg.calculateGamSq(R=R, eb=True)
     true_gamsq = true_gamsq[6:40:4]
     print('gamsq_e = ',gamsq_e)
     print('true_gamsq = ',true_gamsq)

tests/test_helper.py

@@ -38,6 +38,7 @@ def get_from_wiki(file_name, host=None):
         except ImportError:
             from urllib import urlopen
         import shutil
+        import ssl
 
         print('downloading %s from %s...'%(local_file_name,url))
         # urllib.request.urlretrieve(url,local_file_name)
@@ -49,13 +50,9 @@ def get_from_wiki(file_name, host=None):
         # But that can only be done with urlopen, not urlretrieve.  So, here is the solution.
         # cf. http://stackoverflow.com/questions/7243750/download-file-from-web-in-python-3
         #     http://stackoverflow.com/questions/27835619/ssl-certificate-verify-failed-error
+        context = ssl._create_unverified_context()
         try:
-            import ssl
-            context = ssl._create_unverified_context()
             u = urlopen(url, context=context)
-        except (AttributeError, TypeError):
-            # Note: prior to 2.7.9, there is no such function or even the context keyword.
-            u = urlopen(url)
         except urllib.error.HTTPError as e:
             print('Caught ',e)
             print('Wait 10 sec and try again.')
@@ -161,7 +158,6 @@ def nop():
         pass
 _t = Dummy('nop')
 assert_raises = getattr(_t, 'assertRaises')
-#if sys.version_info > (3,2):
 if False:
     # Note: this should work, but at least sometimes it fails with:
     #    RuntimeError: dictionary changed size during iteration

tests/test_mpi.py

@@ -14,58 +14,48 @@
 from __future__ import print_function
 import unittest
 import sys
-if sys.version_info > (3,0):
-    from mockmpi import mock_mpiexec
+from mockmpi import mock_mpiexec
 
 from test_helper import timer
 from mpi_test import setup, do_mpi_gg, do_mpi_ng, do_mpi_nk, do_mpi_nn, do_mpi_kk, do_mpi_kg
 
-@unittest.skipIf(sys.version_info < (3, 0), "mock_mpiexec doesn't support python 2")
 @timer
 def test_mpi_gg():
     output = __name__ == '__main__'
     mock_mpiexec(4, do_mpi_gg, output)
     mock_mpiexec(1, do_mpi_gg, output)
 
-@unittest.skipIf(sys.version_info < (3, 0), "mock_mpiexec doesn't support python 2")
 @timer
 def test_mpi_ng():
     output = __name__ == '__main__'
     mock_mpiexec(4, do_mpi_ng, output)
     mock_mpiexec(1, do_mpi_ng, output)
 
-@unittest.skipIf(sys.version_info < (3, 0), "mock_mpiexec doesn't support python 2")
 @timer
 def test_mpi_nk():
     output = __name__ == '__main__'
     mock_mpiexec(4, do_mpi_nk, output)
     mock_mpiexec(1, do_mpi_nk, output)
 
-@unittest.skipIf(sys.version_info < (3, 0), "mock_mpiexec doesn't support python 2")
 @timer
 def test_mpi_nn():
     output = __name__ == '__main__'
     mock_mpiexec(4, do_mpi_nn, output)
     mock_mpiexec(1, do_mpi_nn, output)
 
-@unittest.skipIf(sys.version_info < (3, 0), "mock_mpiexec doesn't support python 2")
 @timer
 def test_mpi_kg():
     output = __name__ == '__main__'
     mock_mpiexec(4, do_mpi_kg, output)
     mock_mpiexec(1, do_mpi_kg, output)
 
-@unittest.skipIf(sys.version_info < (3, 0), "mock_mpiexec doesn't support python 2")
 @timer
 def test_mpi_kk():
     output = __name__ == '__main__'
     mock_mpiexec(4, do_mpi_kk, output)
     mock_mpiexec(1, do_mpi_kk, output)
 
 if __name__ == '__main__':
-    if sys.version_info < (3,0):
-        print("mockmpi does not support python 2")
-        exit()
     setup()
     test_mpi_gg()
     test_mpi_ng()