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

ENH: Add placeholder stubs for all sub-modules #17104

Merged
merged 12 commits into from
Sep 7, 2020
Merged

Conversation

BvB93
Copy link
Member

@BvB93 BvB93 commented Aug 19, 2020

This pull request adds placeholder stub files for all (public) modules.

"Fixes" the issue encountered in #17103 by adding placeholder stubs for all currently untyped modules.
Note that all objects within the respective modules are currently typed as Any, so this is more of a duct tape solution until proper stubs are added.

Examples

How the submodules were identified:

In [1]: from types import ModuleType
   ...: from typing import Set
   ...: import numpy as np
   ...:
   ...: def get_submodules(super_module: ModuleType) -> Set[str]:
   ...:     """Return a set of all public sub-modules located in the passed `super_module`."""
   ...:     items = ((name, getattr(super_module, name)) for name in dir(super_module))
   ...:     ret = {name for name, obj in items if not name.startswith("_") and isinstance(obj, ModuleType)}
   ...:     return ret - {"sys", "os", "warnings", "math"}  # builtin module

In [2]: get_submodules(np)
Out[2]:
{'char',
 'compat',
 'core',
 'ctypeslib',
 'emath',
 'fft',
 'lib',
 'linalg',
 'ma',
 'matrixlib',
 'polynomial',
 'random',
 'rec',
 'testing',
 'version'}

Copy link
Member

@person142 person142 left a comment

Choose a reason for hiding this comment

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

I like this; in the future I think we could take it even further by having every attribute in the module enumerated and typed as Any (until we have stubs for it) and removing the __getattr__s. In practice I've noticed that partial stubs tend to cause accidental typos, i.e. things like

def f(x: np.array): ...

It would be nice to have mypy catch those.

numpy/warnings.pyi Outdated Show resolved Hide resolved
reveal_type(np.warnings) # E: ModuleType

# TODO: Remove when annotations have been added to `np.linalg.norm`
reveal_type(np.linalg.norm) # E: Any
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to deal with this one right now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, this just serves as a test case to check that the module-level __getattr__() indeed returns Any.

numpy/__init__.pyi Outdated Show resolved Hide resolved
@bashtage
Copy link
Contributor

Can these be smoothly extended, for example, could a part of random be typed without completing the full module and have that part correctly picked up?

@person142
Copy link
Member

Can these be smoothly extended, for example, could a part of random be typed without completing the full module and have that part correctly picked up?

Yup, in fact that's the current state of the main numpy namespace.

@BvB93
Copy link
Member Author

BvB93 commented Aug 19, 2020

I like this; in the future I think we could take it even further by having every attribute in the module enumerated and typed as Any (until we have stubs for it) and removing the __getattr__s.

Good news: turns out the future is today! (see 4af8fd3)

Explicit type annotations (i.e. Any) have now been added for all modules which define __all__.
Note that a couple of them don't seem to define __all__ (which they probably should); those were left untouched.

Examples

The code used for grabbing all __all__s:

from types import ModuleType
from typing import Dict, Optional, List
import numpy as np

def get_submodule_all(super_module: ModuleType) -> Dict[str, Optional[List[str]]]:
    """Return a dict with all (public) sub-modules as keys and their respective `__all__` as values.

    A value is set to `None` if sub-module does not define `__all__`.
    """
    items = ((name, getattr(super_module, name)) for name in dir(super_module))
    superset = {name for name, obj in items if not name.startswith("_") and isinstance(obj, ModuleType)}
    sub_modules = superset - {"sys", "os", "warnings", "math"}  # builtin modules
    return {name: getattr(getattr(super_module, name), '__all__', None) for name in sub_modules}

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

+1 good idea to add placeholder annotations for all public modules.

I would prefer if we didn't blindly annotate everything that doesn't start with an underscore (not sure if that's what you aimed for). Not typing things that are meant to be private, or things that are deprecated, would be better as a signal to users they're in the wrong place.

See numpy/tests/test_public_api.py for things that look public but really aren't.

numpy/__init__.pyi Show resolved Hide resolved
numpy/__init__.pyi Show resolved Hide resolved
@bashtage
Copy link
Contributor

bashtage commented Aug 20, 2020

I'm happy to help out but this has a new learning curve.

I made an attempt on a single function from random and ended up with

from typing import Any, Optional, TypeVar
from numpy.typing import ArrayLike,  _ShapeLike

_ScalarGeneric = TypeVar("_ScalarGeneric", bound=generic)

@overload
def beta(a: _ScalarGeneric, b: _ScalarGeneric, size: None = ...) -> float: ...
@overload
def beta(a: _ScalarGeneric, b: _ScalarGeneric, size: _ShapeLike = ...) -> ndarray: ...
@overload
def beta(a: ArrayLike, b: _ScalarGeneric, size: Optional[_ShapeLike] = ...) -> ndarray: ...
@overload
def beta(a: _ScalarGeneric, b: ArrayLike, size: Optional[_ShapeLike] = ...) -> ndarray: ...
@overload
def beta(a: ArrayLike, b: ArrayLike, size: Optional[_ShapeLike] = ...) -> ndarray: ...

A few questions if you have time:

  1. Is there a better type for float-like but not complex? Or should we type things that will eventually be converted to a float (e.g., a as a float)?
  2. Is there any way to specify a datatype for ArrayLike?
  3. Can _ScalarGeneric be imported from somewhere in pyi files?
  4. Do things like ndarray need to be imported anywhere?

@BvB93
Copy link
Member Author

BvB93 commented Aug 20, 2020

I would prefer if we didn't blindly annotate everything that doesn't start with an underscore (not sure if that's what you aimed for). Not typing things that are meant to be private, or things that are deprecated, would be better as a signal to users they're in the wrong place.

These annotations will definitely have to be refined in the future (they are annotated as Any after all).
Using the module-level __all__ allows for some quick, and reasonably accurate, identification of all public objects. I'd say lets wait with worrying about deprecation until we start replacing the Anys with proper annotations.

numpy/ctypeslib.pyi Outdated Show resolved Hide resolved
numpy/ctypeslib.pyi Outdated Show resolved Hide resolved
@BvB93
Copy link
Member Author

BvB93 commented Aug 20, 2020

@bashtage

Is there a better type for float-like but not complex? Or should we type things that will eventually be converted to a float (e.g., a as a float)?

If #17117 gets merged then typing them as float will be sufficient; otherwise I'd stick to something like _FloatLike = Union[float, np.floating, np.integer].

Is there any way to specify a datatype for ArrayLike?

#16759 is currently working on making ndarray generic with respect to the data type, but it's very much a work in progress. For now I'd just stick to ArrayLike.

Can _ScalarGeneric be imported from somewhere in pyi files?

from numpy import _ScalarGeneric should do the trick. Note that this won't work during runtime; only use it when type checking.

Do things like ndarray need to be imported anywhere?

They do; you can just use from numpy import ndarray.

@charris charris merged commit dec8879 into numpy:master Sep 7, 2020
@charris
Copy link
Member

charris commented Sep 7, 2020

Thanks @BvB93 .

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

6 participants