PR: Follow the Flask plugin model instead of namespace packages #3210

Nodd · 2016-06-07T17:09:59Z

Here is a quick prototype for loading plugins using the flask model.

Fixes #3150.
I went with the two different prefixes because it was simpler, it can be changed in the future.

I still have to look precisely at how to load a module, because from what I saw more functions were deprecated in python 3.5.

TODO:

Test user plugins (in ~/.spyder)
Test pip installed plugins
Remove the removal of whitespace

ccordoba12 · 2016-06-07T17:17:22Z

@Nodd, thanks a lot for working on this one!

Quick comments:

I'd prefer that we name our plugins like spyder_ui_breakpoints instead of spyderui_breakpoints. I think it's clearer.
Please revert blank removals, unless they are in the area of influence of your change.
Why loading code needs to be revisited again?

ccordoba12 · 2016-06-07T17:21:20Z

Or maybe spy_ui_breakpoints to make things shorter ;-)

goanpeca · 2016-06-07T20:21:19Z

@Nodd and @ccordoba12 I dont think adding the ui or io is worth it. IO things are just plugins... so making the difference by name is really just making the names ugly and long.

We should just add some variable in the init of the plugin like

from spyderlib.plugin import IO_PLUGIN, UI_PLUGIN

PLUGIN_TYPES = [IO_PLUGIN, UI_PLUGIN]  # for a plugin that does the two things?
...

And have short names like (pip/conda package and module name):

spyder-autopep8 -> spyder_autopep8
spyder-conda-manager -> spyder_conda_manager
spyder-r -> spyder_r

...

Nodd · 2016-06-07T20:51:30Z

@ccordoba12

I hesitated between the two, the additional underscore make it look very long, but I agree it's clearer.
Raaah, these freaking whitespaces ! We should burn them all ! OK ok, I'll do it.
It's nothing big, right now I commented everything to test things on my machine easily. The code is the same but I'll double check if it's applied to the correct python versions.

@goanpeca From a certain point of view IO plugins are just plugins because they are additions to spyder, but they are actually very different from UI plugins:

they are used exclusively to load new file format, and have nothing to do with anything else
they are loaded during the imports (before spyder's initialization) and are considered as constants in multiple parts of the codebase

Also the big problem with differentiating by module variables (apart from being a little magical) is that you have to load the plugin to check which kind it is. Considering how they are loaded and used very differently internally, I sill think it makes sense to differentiate them explicitly in their names. To be honest if we decide to refactor the way IO plugins are loaded, someone else will have to do it because I won't be able to spend time on this.

To keep names shorter, I'd be in favor of having IO plugins starting with spyder_io_ and generic plugins starting with spyder_. I expect that there will be more generic plugins than io plugins in the future. To be exhaustive I'll add that it prevents generic plugin names from starting with io, but I don't think it'll be a problem. What do you prefer ?
In any case io/ui needs to be in the module name only, not necessarily the plugin name itself. spyder-myplugin can install the module spyder_ui_myplugin.

goanpeca · 2016-06-07T21:29:12Z

@Nodd, I understand how it works and how they load differently and how they might load differently. But IO plugins was just a special case of a general plugin that extends the variable explorer... so in a sense is a normal plugin that extends that functionality. It was not coded that way for whatever reason and it starts in a different part of the application for whatever reason, and trying to compensate for that seems like a bad idea.

In general every normal plugin could/should be extendable.

In any case io/ui needs to be in the module name only, not necessarily the plugin name itself. spyder-myplugin can install the module spyder_ui_myplugin.

I just dislike this very much and find very useful that modules and packages names relate to each other.

I agree that this might be out of the scope of this PR, but I dont see any good reason to maintain the current way IO plugins load/work

Nodd · 2016-06-07T21:50:12Z

It's not "whatever reason". On one side a plugin is a function to load a file with a specific format and extension. On the other side a plugin has a complex API and explicitly needs a dockwidget (a menu-entry plugin needs a hack currently). Even if we name them both "plugins" they have a very different goal (I'm not even speaking about implementation).

It's not just a naming problem, having 2 different interfaces for plugins is weird. I think that the most correct way would be to integrate IO plugins into UI plugins (maybe with a helper class to inherit from ?), feel free to break everything ! 😉

To keep names shorter, I'd be in favor of having IO plugins starting with spyder_io_ and generic plugins starting with spyder_.

What do you think about this ? This way in the future we could change IO plugins only (merging them into ui plugins) while being backward compatible with ui plugins, and keeping the names clean.

goanpeca · 2016-06-07T21:58:38Z

It's not just a naming problem, having 2 different interfaces for plugins is weird. I think that the most correct way would be to integrate IO plugins into UI plugins (maybe with a helper class to inherit from ?), feel free to break everything ! 😉

Yes, I would like to have a single plugin interface, that is the whole point :-)

What do you think about this ? This way in the future we could change IO plugins only (merging them into ui plugins) while being backward compatible with ui plugins, and keeping the names clean.

Yes perfect. I would say the approach to take after this PR is:

Replicate the IO functionality in normal plugins
Allow for easy (io) plugin creation without needing a dockwidget (maybe?)
Allow for a standard way of adding entries to menus (this last one requires adding some sort of groups to menu entries)
Add extension points to the current widgets?

Nodd · 2016-06-07T23:10:05Z

WTF git ? I merged nothing !

goanpeca · 2016-06-07T23:14:13Z

??

Nodd · 2016-06-07T23:23:16Z

All my commits were removed from the PR...
Edit: OK I got it back.

ccordoba12 · 2016-06-08T00:16:39Z

@Nodd, it seems you changed all line endings of several files :-)

Nodd · 2016-06-08T00:25:06Z

😭 I edited some files on windows... I tought it was correctly configured, but it seems I was wrong.

Nodd · 2016-06-08T07:50:01Z

I put the EOL back to the mess they where. I still have to re-mess the whitespaces !

Nodd · 2016-06-08T08:42:48Z

OK, everything should work now ! I also updated spyder.line_profiler and spyder.autopep8 (in my repo, master branch) if you want to test external plugins.

ccordoba12 · 2016-06-08T13:36:08Z

Thanks a lot Joseph! We definitely need to move to normalize line endings!!

Could prepare a new PR about that? I promise to merge it and adapt all
our team PRs accordingly. Thanks :-)

El 08/06/16 a las 03:42, Joseph Martinot-Lagarde escribió:

OK, everything should work now ! I also updated spyder.line_profiler
and spyder.autopep8 (in my repo, master branch) if you want to test
external plugins.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3210 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAWS7U63N7oqKJlSbvRW2L0WqUFuIST0ks5qJoCIgaJpZM4IwJmR.

Nodd · 2016-06-08T14:15:03Z

line_profiler (the plugin) is still buggy, I ported memory_profiler but it's buggy too. You can test the vim plugin, I just did some work on it.

Nodd · 2016-06-09T22:09:47Z

I did some cleanups in the plugins, they should all work now. You have to use the versions from https://github.com/Nodd?tab=repositories, I'll push to spyder-ide when this PR is merged.

Nodd · 2016-06-09T22:13:56Z

... and I'm slowly starting to hate QuantifiedCode. I like nice-looking code, but this thing is so mechanical that the few useful warnings are lost in the noise.
On this specific PR it should detect when the files are moved around and not modified.

ccordoba12 · 2016-06-09T22:43:33Z

@Nodd, is this one ready?

I think the only thing left is to fix setup.py to package the new spyder_* directories instead of spyplugins, but I can do that after merging this one.

Nodd · 2016-06-09T23:04:02Z

I updated the gettext scripts and setup.py (without testing the changes...). Otherwise it should be ready, I don't know if someone tested on another configuration than my machine.

ccordoba12 · 2016-06-10T00:15:47Z

spyderlib/otherplugins.py

-    loads its submodules."""
-    namespace_path = osp.join(plugin_path, base_namespace, plugins_namespace)
+PLUGIN_PREFIX = "spyder_"
+IO_PREFIX = PLUGIN_PREFIX + "io_"


Question here: What's the name for io plugins according to this? Is it spyder_io_foo or spyderio_foo?

I say that because you named our io plugins like spyderio_foo, so those names or this line should be changed :-)

spyder_io_foo. I forgot to rename the files.

ccordoba12 · 2016-06-10T20:25:05Z

setup.py

+                    'spyder_profiler': get_package_data('spyder_profiler', EXTLIST),
+                    'spyder_pylint': get_package_data('spyder_pylint', EXTLIST),
+                    'spyderio_dcm': get_package_data('spyderio_dcm', EXTLIST),
+                    'spyderio_hdf5': get_package_data('spyderio_hdf5', EXTLIST),


@Nodd, these lines also need to be changed to spyder_io_foo :-)

ccordoba12 · 2016-06-10T20:26:08Z

Except for a minor detail in setup.py this one is ready to go.

Nodd · 2016-06-10T22:28:21Z

Stupid me, I modified setup.py before renaming the files.
The travis failure seems like a bug in qtpy with Qt5, it tries to import PySide instead

ccordoba12 · 2016-06-11T00:27:24Z

The problem in Travis had to do with new conda packages for PyQt5. They had the wrong dependencies, so I removed them for now :-)

I have to recompile Qt5 to fix that problem.

Nodd · 2016-06-11T19:27:39Z

I updated the master branch of all "my" plugins in spyder-ide so that they are compatible. Note that it's not the case for spyder.example ! (and spyder.terminado but this one is empty)

TWAC · 2016-07-12T15:52:13Z

After this merge I am not getting plugins from bdist_wheel.
It looks like get_packages() in setup.py still looks for the spyplugins directory.

Nodd · 2016-07-18T13:22:52Z

@TWAC You're right, thank you for noticing. I sent a PR to correct this.

Prototype for flask plugin model

ccb87b0

Nodd added this to the v3.0beta4 milestone Jun 7, 2016

Nodd assigned Nodd and goanpeca Jun 7, 2016

Nodd added 2 commits June 8, 2016 00:34

Use non deprecated function to load plugins

3e4052b

Load generic plugins from spyder_

4bf57a3

Nodd closed this Jun 7, 2016

Nodd force-pushed the flask_plugin_model branch from ccb87b0 to c070e4e Compare June 7, 2016 23:08

Nodd reopened this Jun 7, 2016

Fix travis test for plugins

e4d83bd

Nodd added 2 commits June 8, 2016 09:47

Back to windows EOL

8eefd4b

More windows EOL

94ada11

Nodd added 4 commits June 8, 2016 09:51

profiler.py had Linux EOL ?

d0b7200

Simplify user plugin management

d9c476a

Plugin loading cosmetics

d7f5871

Re-add whitespaces (and remove useless global introduced by mistake)

48d61a2

Nodd added the type:Enhancement label Jun 8, 2016

Nodd added the component:Plugins label Jun 8, 2016

Nodd changed the title ~~[WIP] flask plugin model~~ PR: flask plugin model Jun 9, 2016

ccordoba12 modified the milestones: v3.0rc1, v3.0beta4 Jun 9, 2016

ccordoba12 changed the title ~~PR: flask plugin model~~ PR: Follow the Flask plugin model instead of namespace packages Jun 9, 2016

Nodd added 2 commits June 10, 2016 01:00

Update gettext scripts for new plugin structure

e508c66

Update setup.py for new plugin structure

ed61bac

ccordoba12 reviewed Jun 10, 2016
View reviewed changes

Rename io plugins

637e27d

ccordoba12 reviewed Jun 10, 2016
View reviewed changes

Use correct name for io plugins in setup.py

3b3c9c0

Nodd mentioned this pull request Jun 10, 2016

Ignore case for QT_API env variable in qtpy submodules spyder-ide/qtpy#33

Merged

ccordoba12 merged commit e4959e5 into spyder-ide:master Jun 11, 2016

Nodd deleted the flask_plugin_model branch July 18, 2016 13:11

Nodd mentioned this pull request Jul 18, 2016

Re-include plugins in setup.py packages #3300

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Follow the Flask plugin model instead of namespace packages #3210

PR: Follow the Flask plugin model instead of namespace packages #3210

Nodd commented Jun 7, 2016 •

edited

Loading

ccordoba12 commented Jun 7, 2016

ccordoba12 commented Jun 7, 2016

goanpeca commented Jun 7, 2016 •

edited

Loading

Nodd commented Jun 7, 2016

goanpeca commented Jun 7, 2016 •

edited

Loading

Nodd commented Jun 7, 2016

goanpeca commented Jun 7, 2016 •

edited

Loading

Nodd commented Jun 7, 2016

goanpeca commented Jun 7, 2016

Nodd commented Jun 7, 2016 •

edited

Loading

ccordoba12 commented Jun 8, 2016

Nodd commented Jun 8, 2016

Nodd commented Jun 8, 2016

Nodd commented Jun 8, 2016

ccordoba12 commented Jun 8, 2016

Nodd commented Jun 8, 2016 •

edited

Loading

Nodd commented Jun 9, 2016

Nodd commented Jun 9, 2016

ccordoba12 commented Jun 9, 2016

Nodd commented Jun 9, 2016

ccordoba12 Jun 10, 2016

Nodd Jun 10, 2016

ccordoba12 Jun 10, 2016

ccordoba12 commented Jun 10, 2016

Nodd commented Jun 10, 2016 •

edited

Loading

ccordoba12 commented Jun 11, 2016

Nodd commented Jun 11, 2016

TWAC commented Jul 12, 2016

Nodd commented Jul 18, 2016

PR: Follow the Flask plugin model instead of namespace packages #3210

PR: Follow the Flask plugin model instead of namespace packages #3210

Conversation

Nodd commented Jun 7, 2016 • edited Loading

ccordoba12 commented Jun 7, 2016

ccordoba12 commented Jun 7, 2016

goanpeca commented Jun 7, 2016 • edited Loading

Nodd commented Jun 7, 2016

goanpeca commented Jun 7, 2016 • edited Loading

Nodd commented Jun 7, 2016

goanpeca commented Jun 7, 2016 • edited Loading

Nodd commented Jun 7, 2016

goanpeca commented Jun 7, 2016

Nodd commented Jun 7, 2016 • edited Loading

ccordoba12 commented Jun 8, 2016

Nodd commented Jun 8, 2016

Nodd commented Jun 8, 2016

Nodd commented Jun 8, 2016

ccordoba12 commented Jun 8, 2016

Nodd commented Jun 8, 2016 • edited Loading

Nodd commented Jun 9, 2016

Nodd commented Jun 9, 2016

ccordoba12 commented Jun 9, 2016

Nodd commented Jun 9, 2016

ccordoba12 Jun 10, 2016

Choose a reason for hiding this comment

Nodd Jun 10, 2016

Choose a reason for hiding this comment

ccordoba12 Jun 10, 2016

Choose a reason for hiding this comment

ccordoba12 commented Jun 10, 2016

Nodd commented Jun 10, 2016 • edited Loading

ccordoba12 commented Jun 11, 2016

Nodd commented Jun 11, 2016

TWAC commented Jul 12, 2016

Nodd commented Jul 18, 2016

Nodd commented Jun 7, 2016 •

edited

Loading

goanpeca commented Jun 7, 2016 •

edited

Loading

goanpeca commented Jun 7, 2016 •

edited

Loading

goanpeca commented Jun 7, 2016 •

edited

Loading

Nodd commented Jun 7, 2016 •

edited

Loading

Nodd commented Jun 8, 2016 •

edited

Loading

Nodd commented Jun 10, 2016 •

edited

Loading