KEMBAR78
User-plugin system by chadrik · Pull Request #3512 · python/mypy · GitHub
Skip to content

Conversation

@chadrik
Copy link
Contributor

@chadrik chadrik commented Jun 8, 2017

Here's a design sketch for #1240.

Some thoughts, copied from there:

Plugin discovery options

  • A. by file path. e.g. /path/to/myplugin.py. could also extend this with a MYPY_PLUGIN_PATH
    • pro: easier to write test cases (I discovered that placing a file on the PYTHONPATH within the tests was difficult, likely by design)
    • con: can't use pip to install plugins
  • B. by dotted path: e.g. package.module
    • pro: easy for users to create pip-installable plugins
    • con: adding plugin modules and their requirements to the PYTHONPATH could interfere with type checking?
  • C: setuptools entry points. e.g.:
    setup(
        entry_points={
            'mypy.userplugin': ['my_plugin = my_module.plugin:register_plugin']
        }
    )

Sub-Options

  • shall we always look for an object within the module with a designated named, e.g. Plugin, or make this configurable as well? e.g. package.module.MyPlugin or /path/to/myplugin.py:MyPlugin (Note: I've already written some functionality for the latter in my hooks branch)
  • do user plugins need to inherit from mypy.plugin.Plugin?

Plugin chainability options

  • A. aggregate all user plugins into a single uber-plugin instance.
    • each method on this aggregate plugin would cycle through its children in order until one returns a non-None result. we could then cache the mapping from feature (e.g. 'typing.Mapping.get') to user-plugin instance to speed up future lookups.
    • this is compatible with the current design which passes a single Plugin instance around.
  • B. register a plugin per feature. this allows you to replace the search with a fast dictionary lookup, as well as detect up-front when two plugins contend for the same feature.

Right now I'm using discovery option B and chainability option A, but this is just a starting point for debate.

self.quick_and_dirty = False

# plugins
self.plugins = [] # type: List[PluginRegistry]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is producing an error when running the tests that I don't understand:

mypy/options.py:121: error: Invalid type "mypy.plugin.PluginRegistry"

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that this is because of a reference cycle. There is a known bug that mypy does not treat named tuples in cycles correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads up. I suspected a cycle too, but knew that they seemed to be working in general. I'll see if I can resolve the cycle elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

Choose a reason for hiding this comment

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

Just for information here is the issue about cycles #3054

@chadrik
Copy link
Contributor Author

chadrik commented Jun 8, 2017

Odd that only 3.5.1 seems to have a problem importing typing.Type on travis.

chadrik added 3 commits June 8, 2017 18:00
Two advantages:
- simplifies mypy.build
- related to instantiating user plugins will occur sooner (in main instead of build)
@JelleZijlstra
Copy link
Member

The typing module is provisional and some features were added halfway through the 3.5 series. I believe we test 3.5.1 because that's what Dropbox uses internally.

@gvanrossum
Copy link
Member

The typing module is provisional and some features were added halfway through the 3.5 series. I believe we test 3.5.1 because that's what Dropbox uses internally.

Yes that's exactly right.

This provides a bit more flexibility and also let's us get around the fact that typing.Type does not exist in python 3.5.1.
@chadrik
Copy link
Contributor Author

chadrik commented Jun 9, 2017

Ok, worked around it.

@gvanrossum gvanrossum added the topic-plugins The plugin API and ideas for new plugins label Jun 13, 2017
@JukkaL
Copy link
Collaborator

JukkaL commented Jun 14, 2017

I assume that this can be closed now that #3517 was merged? If there are features not included in #3517 they can potentially be re-engineered as separate PRs on top of #3517.

@JukkaL
Copy link
Collaborator

JukkaL commented Aug 15, 2017

@chadrik Closing this now. If you have further improvements to the plugin system, please send them as new PRs.

@JukkaL JukkaL closed this Aug 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic-plugins The plugin API and ideas for new plugins

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants