Merge pull request #14454 from rgommers/test-public-modules

MAINT: add test to prevent new public-looking modules being added

7 files changed, 421 insertions, 24 deletions

diff --git a/numpy/__init__.py b/numpy/__init__.py
index 07d67945c..fef8245de 100644
--- a/numpy/__init__.py
+++ b/numpy/__init__.py
@@ -143,7 +143,9 @@ else:
     from .core import *
     from . import compat
     from . import lib
+    # FIXME: why have numpy.lib if everything is imported here??
     from .lib import *
+
     from . import linalg
     from . import fft
     from . import polynomial
@@ -174,6 +176,14 @@ else:
     __all__.extend(lib.__all__)
     __all__.extend(['linalg', 'fft', 'random', 'ctypeslib', 'ma'])
 
+    # Remove things that are in the numpy.lib but not in the numpy namespace
+    # Note that there is a test (numpy/tests/test_public_api.py:test_numpy_namespace)
+    # that prevents adding more things to the main namespace by accident.
+    # The list below will grow until the `from .lib import *` fixme above is
+    # taken care of
+    __all__.remove('Arrayterator')
+    del Arrayterator
+
     # Filter out Cython harmless warnings
     warnings.filterwarnings("ignore", message="numpy.dtype size changed")
     warnings.filterwarnings("ignore", message="numpy.ufunc size changed")
diff --git a/numpy/lib/__init__.py b/numpy/lib/__init__.py
index c1757150e..906bede37 100644
--- a/numpy/lib/__init__.py
+++ b/numpy/lib/__init__.py
@@ -5,10 +5,15 @@ import math
 from .info import __doc__
 from numpy.version import version as __version__
 
+# Public submodules
+# Note: recfunctions and (maybe) format are public too, but not imported
+from . import mixins
+from . import scimath as emath
+
+# Private submodules
 from .type_check import *
 from .index_tricks import *
 from .function_base import *
-from .mixins import *
 from .nanfunctions import *
 from .shape_base import *
 from .stride_tricks import *
@@ -16,9 +21,7 @@ from .twodim_base import *
 from .ufunclike import *
 from .histograms import *
 
-from . import scimath as emath
 from .polynomial import *
-#import convertcode
 from .utils import *
 from .arraysetops import *
 from .npyio import *
@@ -28,11 +31,10 @@ from .arraypad import *
 from ._version import *
 from numpy.core._multiarray_umath import tracemalloc_domain
 
-__all__ = ['emath', 'math', 'tracemalloc_domain']
+__all__ = ['emath', 'math', 'tracemalloc_domain', 'Arrayterator']
 __all__ += type_check.__all__
 __all__ += index_tricks.__all__
 __all__ += function_base.__all__
-__all__ += mixins.__all__
 __all__ += shape_base.__all__
 __all__ += stride_tricks.__all__
 __all__ += twodim_base.__all__
diff --git a/numpy/lib/format.py b/numpy/lib/format.py
index 3bf818812..1ecd72815 100644
--- a/numpy/lib/format.py
+++ b/numpy/lib/format.py
@@ -173,6 +173,9 @@ from numpy.compat import (
     )
 
 
+__all__ = []
+
+
 MAGIC_PREFIX = b'\x93NUMPY'
 MAGIC_LEN = len(MAGIC_PREFIX) + 2
 ARRAY_ALIGN = 64 # plausible values are powers of 2 between 16 and 4096
diff --git a/numpy/lib/mixins.py b/numpy/lib/mixins.py
index 52ad45b68..f974a7724 100644
--- a/numpy/lib/mixins.py
+++ b/numpy/lib/mixins.py
@@ -5,8 +5,8 @@ import sys
 
 from numpy.core import umath as um
 
-# Nothing should be exposed in the top-level NumPy module.
-__all__ = []
+
+__all__ = ['NDArrayOperatorsMixin']
 
 
 def _disables_array_ufunc(obj):
diff --git a/numpy/ma/version.py b/numpy/ma/version.py
deleted file mode 100644
index a2c5c42a8..000000000
--- a/numpy/ma/version.py
+++ /dev/null
@@ -1,14 +0,0 @@
-"""Version number
-
-"""
-from __future__ import division, absolute_import, print_function
-
-version = '1.00'
-release = False
-
-if not release:
-    from . import core
-    from . import extras
-    revision = [core.__revision__.split(':')[-1][:-1].strip(),
-                extras.__revision__.split(':')[-1][:-1].strip(),]
-    version += '.dev%04i' % max([int(rev) for rev in revision])
diff --git a/numpy/matlib.py b/numpy/matlib.py
index 9e115943a..604ef470b 100644
--- a/numpy/matlib.py
+++ b/numpy/matlib.py
@@ -2,7 +2,7 @@ from __future__ import division, absolute_import, print_function
 
 import numpy as np
 from numpy.matrixlib.defmatrix import matrix, asmatrix
-# need * as we're copying the numpy namespace
+# need * as we're copying the numpy namespace (FIXME: this makes little sense)
 from numpy import *
 
 __version__ = np.__version__
diff --git a/numpy/tests/test_public_api.py b/numpy/tests/test_public_api.py
index df2fc4802..e27d9b404 100644
--- a/numpy/tests/test_public_api.py
+++ b/numpy/tests/test_public_api.py
@@ -2,14 +2,21 @@ from __future__ import division, absolute_import, print_function
 
 import sys
 import subprocess
+import pkgutil
+import types
+import importlib
+import warnings
 
 import numpy as np
+import numpy
 import pytest
+
 try:
     import ctypes
 except ImportError:
     ctypes = None
 
+
 def check_dir(module, module_name=None):
     """Returns a mapping of all objects with the wrong __module__ attribute."""
     if module_name is None:
@@ -27,7 +34,8 @@ def check_dir(module, module_name=None):
     sys.version_info[0] < 3,
     reason="NumPy exposes slightly different functions on Python 2")
 def test_numpy_namespace():
-    # None of these objects are publicly documented.
+    # None of these objects are publicly documented to be part of the main
+    # NumPy namespace (some are useful though, others need to be cleaned up)
     undocumented = {
         'Tester': 'numpy.testing._private.nosetester.NoseTester',
         '_add_newdoc_ufunc': 'numpy.core._multiarray_umath._add_newdoc_ufunc',
@@ -72,7 +80,7 @@ def test_numpy_namespace():
 
 @pytest.mark.parametrize('name', ['testing', 'Tester'])
 def test_import_lazy_import(name):
-    """Make sure we can actually the the modules we lazy load.
+    """Make sure we can actually use the modules we lazy load.
 
     While not exported as part of the public API, it was accessible.  With the
     use of __getattr__ and __dir__, this isn't always true It can happen that
@@ -101,6 +109,7 @@ def test_numpy_fft():
     bad_results = check_dir(np.fft)
     assert bad_results == {}
 
+
 @pytest.mark.skipif(ctypes is None,
                     reason="ctypes not available in this python")
 def test_NPY_NO_EXPORT():
@@ -109,3 +118,390 @@ def test_NPY_NO_EXPORT():
     f = getattr(cdll, 'test_not_exported', None)
     assert f is None, ("'test_not_exported' is mistakenly exported, "
                       "NPY_NO_EXPORT does not work")
+
+
+# Historically NumPy has not used leading underscores for private submodules
+# much.  This has resulted in lots of things that look like public modules
+# (i.e. things that can be imported as `import numpy.somesubmodule.somefile`),
+# but were never intended to be public.  The PUBLIC_MODULES list contains
+# modules that are either public because they were meant to be, or because they
+# contain public functions/objects that aren't present in any other namespace
+# for whatever reason and therefore should be treated as public.
+#
+# The PRIVATE_BUT_PRESENT_MODULES list contains modules that look public (lack
+# of underscores) but should not be used.  For many of those modules the
+# current status is fine.  For others it may make sense to work on making them
+# private, to clean up our public API and avoid confusion.
+PUBLIC_MODULES = ['numpy.' + s for s in [
+    "ctypeslib",
+    "distutils",
+    "distutils.cpuinfo",
+    "distutils.exec_command",
+    "distutils.misc_util",
+    "distutils.log",
+    "distutils.system_info",
+    "doc",
+    "doc.basics",
+    "doc.broadcasting",
+    "doc.byteswapping",
+    "doc.constants",
+    "doc.creation",
+    "doc.dispatch",
+    "doc.glossary",
+    "doc.indexing",
+    "doc.internals",
+    "doc.misc",
+    "doc.structured_arrays",
+    "doc.subclassing",
+    "doc.ufuncs",
+    "dual",
+    "f2py",
+    "fft",
+    "lib",
+    "lib.format",  # was this meant to be public?
+    "lib.mixins",
+    "lib.recfunctions",
+    "lib.scimath",
+    "linalg",
+    "ma",
+    "ma.extras",
+    "ma.mrecords",
+    "matlib",
+    "polynomial",
+    "polynomial.chebyshev",
+    "polynomial.hermite",
+    "polynomial.hermite_e",
+    "polynomial.laguerre",
+    "polynomial.legendre",
+    "polynomial.polynomial",
+    "polynomial.polyutils",
+    "random",
+    "testing",
+    "version",
+]]
+
+
+PUBLIC_ALIASED_MODULES = [
+    "numpy.char",
+    "numpy.emath",
+    "numpy.rec",
+]
+
+
+PRIVATE_BUT_PRESENT_MODULES = ['numpy.' + s for s in [
+    "compat",
+    "compat.py3k",
+    "conftest",
+    "core",
+    "core.arrayprint",
+    "core.defchararray",
+    "core.einsumfunc",
+    "core.fromnumeric",
+    "core.function_base",
+    "core.getlimits",
+    "core.info",
+    "core.machar",
+    "core.memmap",
+    "core.multiarray",
+    "core.numeric",
+    "core.numerictypes",
+    "core.overrides",
+    "core.records",
+    "core.shape_base",
+    "core.umath",
+    "core.umath_tests",
+    "distutils.ccompiler",
+    "distutils.command",
+    "distutils.command.autodist",
+    "distutils.command.bdist_rpm",
+    "distutils.command.build",
+    "distutils.command.build_clib",
+    "distutils.command.build_ext",
+    "distutils.command.build_py",
+    "distutils.command.build_scripts",
+    "distutils.command.build_src",
+    "distutils.command.config",
+    "distutils.command.config_compiler",
+    "distutils.command.develop",
+    "distutils.command.egg_info",
+    "distutils.command.install",
+    "distutils.command.install_clib",
+    "distutils.command.install_data",
+    "distutils.command.install_headers",
+    "distutils.command.sdist",
+    "distutils.compat",
+    "distutils.conv_template",
+    "distutils.core",
+    "distutils.extension",
+    "distutils.fcompiler",
+    "distutils.fcompiler.absoft",
+    "distutils.fcompiler.compaq",
+    "distutils.fcompiler.environment",
+    "distutils.fcompiler.g95",
+    "distutils.fcompiler.gnu",
+    "distutils.fcompiler.hpux",
+    "distutils.fcompiler.ibm",
+    "distutils.fcompiler.intel",
+    "distutils.fcompiler.lahey",
+    "distutils.fcompiler.mips",
+    "distutils.fcompiler.nag",
+    "distutils.fcompiler.none",
+    "distutils.fcompiler.pathf95",
+    "distutils.fcompiler.pg",
+    "distutils.fcompiler.sun",
+    "distutils.fcompiler.vast",
+    "distutils.from_template",
+    "distutils.info",
+    "distutils.intelccompiler",
+    "distutils.lib2def",
+    "distutils.line_endings",
+    "distutils.mingw32ccompiler",
+    "distutils.msvccompiler",
+    "distutils.npy_pkg_config",
+    "distutils.numpy_distribution",
+    "distutils.pathccompiler",
+    "distutils.unixccompiler",
+    "f2py.auxfuncs",
+    "f2py.capi_maps",
+    "f2py.cb_rules",
+    "f2py.cfuncs",
+    "f2py.common_rules",
+    "f2py.crackfortran",
+    "f2py.diagnose",
+    "f2py.f2py2e",
+    "f2py.f2py_testing",
+    "f2py.f90mod_rules",
+    "f2py.func2subr",
+    "f2py.info",
+    "f2py.rules",
+    "f2py.use_rules",
+    "fft.helper",
+    "lib.arraypad",
+    "lib.arraysetops",
+    "lib.arrayterator",
+    "lib.financial",
+    "lib.function_base",
+    "lib.histograms",
+    "lib.index_tricks",
+    "lib.info",
+    "lib.nanfunctions",
+    "lib.npyio",
+    "lib.polynomial",
+    "lib.shape_base",
+    "lib.stride_tricks",
+    "lib.twodim_base",
+    "lib.type_check",
+    "lib.ufunclike",
+    "lib.user_array",  # note: not in np.lib, but probably should just be deleted
+    "lib.utils",
+    "linalg.info",
+    "linalg.lapack_lite",
+    "linalg.linalg",
+    "ma.bench",
+    "ma.core",
+    "ma.testutils",
+    "ma.timer_comparison",
+    "matrixlib",
+    "matrixlib.defmatrix",
+    "random.bit_generator",
+    "random.bounded_integers",
+    "random.common",
+    "random.generator",
+    "random.info",
+    "random.mt19937",
+    "random.mtrand",
+    "random.pcg64",
+    "random.philox",
+    "random.sfc64",
+    "testing.decorators",
+    "testing.noseclasses",
+    "testing.nosetester",
+    "testing.print_coercion_tables",
+    "testing.utils",
+]]
+
+
+def is_unexpected(name):
+    """Check if this needs to be considered."""
+    if '._' in name or '.tests' in name or '.setup' in name:
+        return False
+
+    if name in PUBLIC_MODULES:
+        return False
+
+    if name in PUBLIC_ALIASED_MODULES:
+        return False
+
+    if name in PRIVATE_BUT_PRESENT_MODULES:
+        return False
+
+    return True
+
+
+# These are present in a directory with an __init__.py but cannot be imported
+# code_generators/ isn't installed, but present for an inplace build
+SKIP_LIST = [
+    "numpy.core.code_generators",
+    "numpy.core.code_generators.genapi",
+    "numpy.core.code_generators.generate_umath",
+    "numpy.core.code_generators.ufunc_docstrings",
+    "numpy.core.code_generators.generate_numpy_api",
+    "numpy.core.code_generators.generate_ufunc_api",
+    "numpy.core.code_generators.numpy_api",
+    "numpy.core.cversions",
+    "numpy.core.generate_numpy_api",
+    "numpy.distutils.msvc9compiler",
+]
+
+
+def test_all_modules_are_expected():
+    """
+    Test that we don't add anything that looks like a new public module by
+    accident.  Check is based on filenames.
+    """
+
+    modnames = []
+    for _, modname, ispkg in pkgutil.walk_packages(path=np.__path__,
+                                                   prefix=np.__name__ + '.',
+                                                   onerror=None):
+        if is_unexpected(modname) and modname not in SKIP_LIST:
+            # We have a name that is new.  If that's on purpose, add it to
+            # PUBLIC_MODULES.  We don't expect to have to add anything to
+            # PRIVATE_BUT_PRESENT_MODULES.  Use an underscore in the name!
+            modnames.append(modname)
+
+    if modnames:
+        raise AssertionError("Found unexpected modules: {}".format(modnames))
+
+
+# Stuff that clearly shouldn't be in the API and is detected by the next test
+# below
+SKIP_LIST_2 = [
+    'numpy.math',
+    'numpy.distutils.log.sys',
+    'numpy.distutils.system_info.copy',
+    'numpy.distutils.system_info.distutils',
+    'numpy.distutils.system_info.log',
+    'numpy.distutils.system_info.os',
+    'numpy.distutils.system_info.platform',
+    'numpy.distutils.system_info.re',
+    'numpy.distutils.system_info.shutil',
+    'numpy.distutils.system_info.subprocess',
+    'numpy.distutils.system_info.sys',
+    'numpy.distutils.system_info.tempfile',
+    'numpy.distutils.system_info.textwrap',
+    'numpy.distutils.system_info.warnings',
+    'numpy.doc.constants.re',
+    'numpy.doc.constants.textwrap',
+    'numpy.lib.emath',
+    'numpy.lib.math',
+    'numpy.matlib.char',
+    'numpy.matlib.rec',
+    'numpy.matlib.emath',
+    'numpy.matlib.math',
+    'numpy.matlib.linalg',
+    'numpy.matlib.fft',
+    'numpy.matlib.random',
+    'numpy.matlib.ctypeslib',
+    'numpy.matlib.ma'
+]
+
+
+def test_all_modules_are_expected_2():
+    """
+    Method checking all objects. The pkgutil-based method in
+    `test_all_modules_are_expected` does not catch imports into a namespace,
+    only filenames.  So this test is more thorough, and checks this like:
+
+        import .lib.scimath as emath
+
+    To check if something in a module is (effectively) public, one can check if
+    there's anything in that namespace that's a public function/object but is
+    not exposed in a higher-level namespace.  For example for a `numpy.lib`
+    submodule::
+
+        mod = np.lib.mixins
+        for obj in mod.__all__:
+            if obj in np.__all__:
+                continue
+            elif obj in np.lib.__all__:
+                continue
+
+            else:
+                print(obj)
+
+    """
+
+    def find_unexpected_members(mod_name):
+        members = []
+        module = importlib.import_module(mod_name)
+        if hasattr(module, '__all__'):
+            objnames = module.__all__
+        else:
+            objnames = dir(module)
+
+        for objname in objnames:
+            if not objname.startswith('_'):
+                fullobjname = mod_name + '.' + objname
+                if isinstance(getattr(module, objname), types.ModuleType):
+                    if is_unexpected(fullobjname):
+                        if fullobjname not in SKIP_LIST_2:
+                            members.append(fullobjname)
+
+        return members
+
+    unexpected_members = find_unexpected_members("numpy")
+    for modname in PUBLIC_MODULES:
+        unexpected_members.extend(find_unexpected_members(modname))
+
+    if unexpected_members:
+        raise AssertionError("Found unexpected object(s) that look like "
+                             "modules: {}".format(unexpected_members))
+
+
+def test_api_importable():
+    """
+    Check that all submodules listed higher up in this file can be imported
+
+    Note that if a PRIVATE_BUT_PRESENT_MODULES entry goes missing, it may
+    simply need to be removed from the list (deprecation may or may not be
+    needed - apply common sense).
+    """
+    def check_importable(module_name):
+        try:
+            importlib.import_module(module_name)
+        except (ImportError, AttributeError):
+            return False
+
+        return True
+
+    module_names = []
+    for module_name in PUBLIC_MODULES:
+        if not check_importable(module_name):
+            module_names.append(module_name)
+
+    if module_names:
+        raise AssertionError("Modules in the public API that cannot be "
+                             "imported: {}".format(module_names))
+
+    for module_name in PUBLIC_ALIASED_MODULES:
+        try:
+            eval(module_name)
+        except AttributeError:
+            module_names.append(module_name)
+
+    if module_names:
+        raise AssertionError("Modules in the public API that were not "
+                             "found: {}".format(module_names))
+
+    with warnings.catch_warnings(record=True) as w:
+        warnings.filterwarnings('always', category=DeprecationWarning)
+        warnings.filterwarnings('always', category=ImportWarning)
+        for module_name in PRIVATE_BUT_PRESENT_MODULES:
+            if not check_importable(module_name):
+                module_names.append(module_name)
+
+    if module_names:
+        raise AssertionError("Modules that are not really public but looked "
+                             "public and can not be imported: "
+                             "{}".format(module_names))