scipy / scipy_doctest

Floating-point aware doctesting
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

Ensure imported objects are not collected for doctesting #98

Closed Sheila-nk closed 11 months ago

Sheila-nk commented 12 months ago

Imported objects seem to be collected for doctesting:

# scipy/datasets/_fetchers.py

from numpy import array, frombuffer, load
scipy/datasets/_fetchers.py::scipy.datasets._fetchers.array FAILED                                                                                                                                   
scipy/datasets/_fetchers.py::scipy.datasets._fetchers.frombuffer FAILED                                                                                                                              
scipy/datasets/_fetchers.py::scipy.datasets._fetchers.load FAILED  
# scipy/fft/_basic.py

from scipy._lib.uarray import generate_multimethod, Dispatchable
scipy/fft/_basic.py::scipy.fft._basic.Dispatchable FAILED                                                                                                                                            
scipy/fft/_basic.py::scipy.fft._basic.generate_multimethod FAILED    
  • Why is array event picked up here? It is not in the public API, so should not be even looked at with strategy="api".
    [doctest] scipy.datasets._fetchers.array ___________________
    EXAMPLE LOCATION UNKNOWN, not showing all tests of that example
    ??? >>> np.array([1, 2, 3])
    UNEXPECTED EXCEPTION: NameError("name 'np' is not defined")
    Traceback (most recent call last):
    File "/home/circleci/.pyenv/versions/3.9.18/lib/python3.9/doctest.py", line 1334, in __run
    exec(compile(example.source, filename, "single",
    File "<doctest scipy.datasets._fetchers.array[0]>", line 1, in <module>
    NameError: name 'np' is not defined
    /home/circleci/.pyenv/versions/3.9.18/lib/python3.9/site-packages/numpy/__init__.py:None: UnexpectedException

    I would compare the runs with pytest and manual, to see what's going wrong.

Originally posted by @ev-br in https://github.com/scipy/scipy/issues/19242#issuecomment-1722192490

Sheila-nk commented 12 months ago

From my understanding, find_doctests utilizes the get_public_objects function which calls get_all_list to get a list of public objects specified by the modules __all__ attribute. If the __all__ attribute is not present, it gets the names obtained from the dir(module) function which includes all imported objects.

On printing the all_list returned from the scipy.datasets._fetchers module: ['array', 'ascent', 'data_fetcher', 'electrocardiogram', 'face', 'fetch_data', 'frombuffer', 'load', 'registry', 'registry_urls']

This list includes objects imported from numpy which should not be collected for doctesting.

Sheila-nk commented 12 months ago

@ev-br The problem seems to arise here: https://github.com/ev-br/scpdt/blob/33f3bb1c619f77e92ca5802016fab3b16c505a99/scpdt/util.py#L179

I'm not sure it is necessary to get the module's objects, especially since we eventually append the module to the items list in the find_doctests function: https://github.com/ev-br/scpdt/blob/33f3bb1c619f77e92ca5802016fab3b16c505a99/scpdt/frontend.py#L78

Returning an empty all_list instead, if the __all__ attribute is not present in the module seems to solve the problem. What do you think?

Sheila-nk commented 12 months ago

I have noticed these objects are doctested by the plugin but not by refguide-check, even though they are listed in the modules' __all__ attributes:

scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miDOUBLE PASSED                                                                                                                                      [ 13%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miINT16 PASSED                                                                                                                                       [ 13%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miINT32 PASSED                                                                                                                                       [ 13%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miSINGLE PASSED                                                                                                                                      [ 13%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miUINT16 PASSED                                                                                                                                      [ 13%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miUINT8 PASSED                                                                                                                                       [ 13%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.mxCHAR_CLASS PASSED                                                                                                                                  [ 14%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.mxFULL_CLASS PASSED                                                                                                                                  [ 14%]
scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.mxSPARSE_CLASS PASSED                                                                                                                                [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miCOMPRESSED PASSED                                                                                                                    [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miINT64 PASSED                                                                                                                         [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miINT8 PASSED                                                                                                                          [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miMATRIX PASSED                                                                                                                        [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miUINT32 PASSED                                                                                                                        [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miUINT64 PASSED                                                                                                                        [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miUTF16 PASSED                                                                                                                         [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miUTF32 PASSED                                                                                                                         [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.miUTF8 PASSED                                                                                                                          [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxCELL_CLASS PASSED                                                                                                                    [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxDOUBLE_CLASS PASSED                                                                                                                  [ 14%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxFUNCTION_CLASS PASSED                                                                                                                [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxINT16_CLASS PASSED                                                                                                                   [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxINT32_CLASS PASSED                                                                                                                   [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxINT64_CLASS PASSED                                                                                                                   [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxINT8_CLASS PASSED                                                                                                                    [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxOBJECT_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxOBJECT_CLASS_FROM_MATRIX_H PASSED                                                                                                    [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxOPAQUE_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxSINGLE_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxSTRUCT_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxUINT16_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxUINT32_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxUINT64_CLASS PASSED                                                                                                                  [ 15%]
scipy/io/matlab/_mio5_params.py::scipy.io.matlab._mio5_params.mxUINT8_CLASS PASSED                                                                                                                   

what are they?

ev-br commented 12 months ago

Returning an empty all_list instead, if the all attribute is not present in the module seems to solve the problem. What do you think?

Yeah, get_public_objects was designed to work with an explicit list of public submodules (cf refguide-check and the old scipy/numpy PRs). Adding this list back is something I'd rather avoid if possible.

This all starts to bring back memories of an undocumented glitch in nose skipping with leading underscore. This was tripping multiple people for several years until nose failed unto non-existence :-). Anyway.

All that said, if not doing the dir(module) fallback fixes things, let's just go with it! Please document though.

ev-br commented 12 months ago

scipy/io/matlab/_mio4.py::scipy.io.matlab._mio4.miDOUBLE PASSED

No idea what they are exactly, but (1) they definitely come from scipy, and (2) as long as all their tests pass, great :-).

Sheila-nk commented 12 months ago

especially since we eventually append the module to the items list in the find_doctests function:

I had misunderstood this implementation: https://github.com/ev-br/scpdt/blob/33f3bb1c619f77e92ca5802016fab3b16c505a99/scpdt/frontend.py#L78 If the item is a module, the recurse parameter is set to False. So this only works if the objects in the module have already been collected for doctesting. It only doctests the "bare" docstring examples.

if inspect.ismodule(item):
            # do not recurse, only inspect the module docstring
            _finder = DTFinder(recurse=False, config=config)
            t = _finder.find(item, name, globs=globs, extraglobs=extraglobs)
ev-br commented 12 months ago

... and the conclusion is, once the misunderstanding is cleared?

Sheila-nk commented 12 months ago

The conclusion is, if strategy=api, it makes sense to only collect public objects explicitly listed in the __all__ attribute.