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

TST: Valid docstring for config_py function show() #17621

Merged
merged 1 commit into from
Oct 26, 2020

Conversation

has2k1
Copy link
Contributor

@has2k1 has2k1 commented Oct 24, 2020

numpy.distutils generates a function __config__.show. This change
makes the docstring of that function complete (with the required
imports) and runnable. This prevents numpy.distutils from adding
doctests that will fail -- if picked-up by the test runner.

Example of failing test: https://travis-ci.org/github/has2k1/scikit-misc/jobs/702785136#L747

numpy.distutils generates a function `__config__.show`. This change
makes the docstring of that function complete (with the required
imports) and runnable. This prevents numpy.distutils from adding
doctests that will fail -- if picked-up by the test runner.
@mattip
Copy link
Member

mattip commented Oct 24, 2020

I am confused. I don't see how the generated skmisc/__config__.py picks up the docstring in the first place (at least the version present in the __config__.py supplied by pip install scikit-misc below).

skmisc/__config__.py ``` # This file is generated by numpy's setup.py # It contains system_info results at the time of building this package. __all__ = ["get_info","show"]

import os
import sys

extra_dll_dir = os.path.join(os.path.dirname(file), '.libs')
if sys.platform == 'win32' and os.path.isdir(extra_dll_dir):
os.environ.setdefault('PATH', '')
os.environ['PATH'] += os.pathsep + extra_dll_dir
blas_mkl_info={}
blis_info={}
openblas_info={'libraries': ['openblas', 'openblas'], 'library_dirs': ['/usr/local/lib'], 'language': 'c', 'define_macros': [('HAVE_CBLAS', None)]}
blas_opt_info={'libraries': ['openblas', 'openblas'], 'library_dirs': ['/usr/local/lib'], 'language': 'c', 'define_macros': [('HAVE_CBLAS', None)]}

def get_info(name):
g = globals()
return g.get(name, g.get(name + "_info", {}))

def show():
for name,info_dict in globals().items():
if name[0] == "_" or type(info_dict) is not type({}): continue
print(name + ":")
if not info_dict:
print(" NOT AVAILABLE")
for k,v in info_dict.items():
v = str(v)
if k == "sources" and len(v) > 200:
v = v[:60] + " ...\n... " + v[-60:]
print(" %s = %s" % (k,v))

@has2k1
Copy link
Contributor Author

has2k1 commented Oct 25, 2020

I don't see how the generated skmisc/__config__.py picks up the docstring in the first place

I think the function with a docstring shipped in v1.19.

@mattip
Copy link
Member

mattip commented Oct 25, 2020

I understand that there is a docstring in numpy.__config__. But how does that get picked up by the skmisc.__config__.py docstring test?

@has2k1
Copy link
Contributor Author

has2k1 commented Oct 25, 2020

The config file (in this case skmisc.__config__.py) is placed in the package root directory. That means if you point your automated test discovery tool at the package root (for example pytest skmisc) and you are also running doctests (e.g. pytest --doctest-modules skmisc or by setting the option a config file), then skmisc/__config__.py will be picked up for testing and the doctests in it will run.

@mattip
Copy link
Member

mattip commented Oct 25, 2020

But as I demonstrated above (repeated below), there are no docstring tests in skmisc/__config__.py, and even if there were it should not be picking up numpy's docstrings.

# This file is generated by numpy's setup.py
# It contains system_info results at the time of building this package.
__all__ = ["get_info","show"]


import os
import sys

extra_dll_dir = os.path.join(os.path.dirname(__file__), '.libs')
if sys.platform == 'win32' and os.path.isdir(extra_dll_dir):
    os.environ.setdefault('PATH', '')
    os.environ['PATH'] += os.pathsep + extra_dll_dir
blas_mkl_info={}
blis_info={}
openblas_info={'libraries': ['openblas', 'openblas'], 'library_dirs': ['/usr/local/lib'], 'language': 'c', 'define_macros': [('HAVE_CBLAS', None)]}
blas_opt_info={'libraries': ['openblas', 'openblas'], 'library_dirs': ['/usr/local/lib'], 'language': 'c', 'define_macros': [('HAVE_CBLAS', None)]}

def get_info(name):
    g = globals()
    return g.get(name, g.get(name + "_info", {}))

def show():
    for name,info_dict in globals().items():
        if name[0] == "_" or type(info_dict) is not type({}): continue
        print(name + ":")
        if not info_dict:
            print("  NOT AVAILABLE")
        for k,v in info_dict.items():
            v = str(v)
            if k == "sources" and len(v) > 200:
                v = v[:60] + " ...\n... " + v[-60:]
            print("    %s = %s" % (k,v))

@pv
Copy link
Member

pv commented Oct 25, 2020

@mattip: old versions of Numpy did not add the docstring, it was added in 1.19.0 (gh-15429), so sdists generated with previous Numpy versions don't have it. I don't remember if pytest is supposed to run tests in modules starting with underscore, but if it does, then this looks like it will happen.

@mattip
Copy link
Member

mattip commented Oct 25, 2020

Perhaps this generated __config__.py should have the name of the package being configured as a template parameter in the docstring instead of numpy

@has2k1
Copy link
Contributor Author

has2k1 commented Oct 26, 2020

@mattip, This is the __config__.py generated by numpy 1.19.0 and above.

# This file is generated by numpy's setup.py
# It contains system_info results at the time of building this package.
__all__ = ["get_info","show"]


import os
import sys

extra_dll_dir = os.path.join(os.path.dirname(__file__), '.libs')

if sys.platform == 'win32' and os.path.isdir(extra_dll_dir):
    if sys.version_info >= (3, 8):
        os.add_dll_directory(extra_dll_dir)
    else:
        os.environ.setdefault('PATH', '')
        os.environ['PATH'] += os.pathsep + extra_dll_dir

blas_mkl_info={}
blis_info={}
openblas_info={'libraries': ['openblas', 'openblas'], 'library_dirs': ['/usr/lib64'], 'language': 'c', 'define_macros': [('HAVE_CBLAS', None)]}
blas_opt_info={'libraries': ['openblas', 'openblas'], 'library_dirs': ['/usr/lib64'], 'language': 'c', 'define_macros': [('HAVE_CBLAS', None)]}

def get_info(name):
    g = globals()
    return g.get(name, g.get(name + "_info", {}))

def show():
    """
    Show libraries in the system on which NumPy was built.

    Print information about various resources (libraries, library
    directories, include directories, etc.) in the system on which
    NumPy was built.

    See Also
    --------
    get_include : Returns the directory containing NumPy C
                  header files.

    Notes
    -----
    Classes specifying the information to be printed are defined
    in the `numpy.distutils.system_info` module.

    Information may include:

    * ``language``: language used to write the libraries (mostly
      C or f77)
    * ``libraries``: names of libraries found in the system
    * ``library_dirs``: directories containing the libraries
    * ``include_dirs``: directories containing library header files
    * ``src_dirs``: directories containing library source files
    * ``define_macros``: preprocessor macros used by
      ``distutils.setup``

    Examples
    --------
    >>> np.show_config()
    blas_opt_info:
        language = c
        define_macros = [('HAVE_CBLAS', None)]
        libraries = ['openblas', 'openblas']
        library_dirs = ['/usr/local/lib']
    """
    for name,info_dict in globals().items():
        if name[0] == "_" or type(info_dict) is not type({}): continue
        print(name + ":")
        if not info_dict:
            print("  NOT AVAILABLE")
        for k,v in info_dict.items():
            v = str(v)
            if k == "sources" and len(v) > 200:
                v = v[:60] + " ...\n... " + v[-60:]
            print("    %s = %s" % (k,v))

@has2k1
Copy link
Contributor Author

has2k1 commented Oct 26, 2020

I don't remember if pytest is supposed to run tests in modules starting with underscore, but if it does, then this looks like it will happen.

Yes it does.

@pv pv merged commit 2dcbc56 into numpy:master Oct 26, 2020
@pv
Copy link
Member

pv commented Oct 26, 2020

Merging, seems straightforward. I'd guess the whole docstring could be revised to not talk about Numpy at all as Matti suggests with a bit more work, but maybe good to just put in a quick fix.

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

Successfully merging this pull request may close these issues.

None yet

3 participants