Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.

ENH: add mypy plugin for more precise ufunc signatures #56

Closed
wants to merge 4 commits into from

Conversation

person142
Copy link
Member

Keeping this as a draft as it needs discussion (if we decide its ok I'll need to add more data on ufunc arities to the plugin).

This adds a mypy plugin that alters ufunc signatures to have the correct arity at type checking time. The tl;dr version is:

np.sin(1, 1)  # Ok on master, but fails with this plugin

The broader issue is:

  • ufuncs are all instances of the same class
  • but their arities differ
  • there's no way to capture that in the current type system

To work around that, the plugin lets us halt each time a numpy.ufunc.__call__ node is reached in the AST and modify the signature. This just does arities, but you can imagine that when ndarray is generic over dtypes then we can potentially do even fancier things like adding overloads for all the appropriate dispatches for the ufunc.

Now, I imagine one concern people might have is "doesn't this lock us into mypy", and the answer is "not really". The plugin is purely opt in-you have to include it in your mypy.ini for it to be activated. So we can easily have the stubs, which describe things as best they can, and a set of plugins that give more precise types for people who happen to be using mypy.

Thoughts?

@@ -3,3 +3,4 @@
np.sin.nin + 'foo' # E: Unsupported operand types
np.sin(1, foo='bar') # E: Unexpected keyword argument
np.sin(1, extobj=['foo', 'foo', 'foo']) # E: incompatible type
np.sin(1, 1) # E: Too many positional arguments
Copy link
Member Author

Choose a reason for hiding this comment

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

The punchline: we statically know that sin takes one argument.

tests/mypy.ini Outdated
@@ -0,0 +1,2 @@
[mypy]
plugins = numpy_ufuncs_plugin
Copy link
Member Author

Choose a reason for hiding this comment

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

We have our own tests opt into using the plugin.

from mypy.plugin import Plugin
from mypy.types import CallableType

UFUNC_ARITIES = {
Copy link
Member Author

Choose a reason for hiding this comment

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

We would need a big dictionary of the ufuncs and their arities.

Copy link
Member

Choose a reason for hiding this comment

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

UFuncs should provide all the information you need for this, no? Such as np.sin.nin, np.sin.nout, and np.sin.signature (which is None for normal ufuncs).

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been assuming it's bad practice to import NumPy instead of working just from the AST, but heck I don't know it seems pretty reasonable here... maybe we should just do it? Worried about triggering https://xkcd.com/292/.

Copy link
Member

Choose a reason for hiding this comment

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

Ohh, I didn't think about that and I guess it is a mypy/typing code smell question and I have no nose for typing :). I suppose we thus need something like a decorator for ufuncs that duplicates some of the information? (Its not like we can get the decorator information to the C-level cleanly probably).
Also, it is not like NumPy is the only one with ufuncs. SciPy will need to use this hook as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Switched to pulling it off the ufunc object in f8c5979. One (and only one!) of the CI builds is failing though...

@person142
Copy link
Member Author

Interestingly, PEP 612:

https://www.python.org/dev/peps/pep-0612/

might also allow us to better type ufuncs by making the ufunc class generic over a parameter specification.

@person142
Copy link
Member Author

And here's a branch

https://github.com/person142/numpy-stubs/tree/ufunc-plugin-with-literal

with a plugin that abuses Literals to allow you to specify ufunc arity by doing something like

from typing_extensions import Literal

sin: ufunc[Literal[1], Literal[1]]
           ^           ^
           nin         nout

So I guess we've got some options:

  • Include the ufunc arity data in the plugin itself
    • Pretty tame
    • Can't handle non-NumPy ufuncs
    • Not very DRY
  • Import NumPy in the plugin
    • Maybe a little sketchy
    • Can't handle non-NumPy ufuncs
    • For some reason fails on 3.7 in CI (but works in 3.6 and 3.8, so might be a bug elsewhere)
  • Abuse literals
    • Pretty out there
    • Can handle non-NumPy ufuncs

@seberg
Copy link
Member

seberg commented Apr 19, 2020

My main concern is that we really should have a solution that will work also for other libraries, and while importing numpy is possibly fine, we should maybe not force the library to do the same?

I am still curious if we cannot do the plugin solution anyway, but provide a way to "expand" the ufuncs, if necessary by telling projects how to create a very simply plugin themselves? The literal solution seems fine, although having Literal in there seems a bit ugly?

@person142
Copy link
Member Author

Closing this; it was a good experiment with plugins (I think I feel more confident that they will be a big part of supporting types in NumPy), but it's not really "production grade".

@person142 person142 closed this Jun 9, 2020
@person142 person142 deleted the ufunc-plugin branch June 9, 2020 15:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants