Spyder: Split all plugins to be in their own modules

Created on 12 Jun 2017  Â·  32Comments  Â·  Source: spyder-ide/spyder

An issue to keep track of spyder modularization:

  • [x] Help #4548
  • [x] Online Help #4565
  • [x] Variable Explorer #4557
  • [x] Find in Files #4569
  • [x] Editor #4975
  • [x] Outline explorer #4812
  • [x] IPython Console #5206
  • [x] Internal Console #5207
  • [x] History #4593
  • [x] Projects (project explorer) #5214
  • [x] Explorer #4974
  • [x] Working Directory ~(will be merged with explorer?)~ #5219
  • [x] Preferences module #5216
  • [x] Fix error in variableexplorer (https://github.com/spyder-ide/spyder/pull/5206#issuecomment-329018836) #5263

External plugins (spyder_breakpoints, spyder_breakpoints, spyder_io_hdf5, spyder_profiler, spyder_pylint)

  • [x] Move back to the plugins folder (also change them to follow the same folder structure): #5276
  • [x] merge translations #5438

Other things:

  • [x] Merge with master and move new files (after 4.0beta2 release) #7725

Other ideas (to further discuss):

  • Add PLUGIN_CLASS to internal plugins, and load them as external plugins are loaded
Techdebt

Most helpful comment

A bit more than one year after @rlaverde started this big refactoring, this is finally ready!!

Gee, I hope to not do it again!!

All 32 comments

Profiler, lint etc... should be brought back to the plugins folder

Profiler, lint etc... should be brought back to the plugins folder

yes, I forgot that

We need a separate issue for after this, cause the idea is that these plugins should really be extending the variable explorer API. Currently the way they load is too weird...

spyder_io_dcm
spyder_io_hdf5

Thanks for opening this one to keep track of this effort.

@rlaverde @ccordoba12 how should we handle the preferences "plugin", its not really a plugin at the moment, more of a core feature of spyder, so we need to move it out of plugins folder?

Thoughts?

Right, we should move all Python modules related to Preferences (e.g.
configdialog.py, rundialog.py, etc) to a different module called
preferences, out of plugins and at the same level of it.

El 12/09/17 a las 09:06, Gonzalo Peña-Castellanos escribió:
>

@rlaverde https://github.com/rlaverde @ccordoba12
https://github.com/ccordoba12 how should we handle the preferences
"plugin", its not really a plugin at the moment, more of a core
feature of spyder, so we need to move it out of plugins folder?

Thoughts?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/spyder-ide/spyder/issues/4591#issuecomment-328863786,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWS7eVJT4UWxcWgDcBofAcAP-qAC0x9ks5sho_2gaJpZM4N3Xj3.

Also @rlaverde what about https://github.com/spyder-ide/spyder/tree/split-plugins/spyder/plugins/tests

That belongs to the findinfiles plugin?

@goanpeca is right, that one is missing.

That file was generated, after merging with master, there are some new files in master that need to me moved to their module (extensions, some tests) I'll do that after merging again with master

Looking at external plugins there's a difference with internal spyder plugins, they have a PLUGIN_CLASS I think that internal plugins should have it too

I think the next step could be to split preferences, each plugin should have an configuration.py with the configuration defaults (i.e. split config/main.py DEFAULTS list), I think that could help to solve #4306

I think the next step could be to split preferences, each plugin should have an configuration.py with the configuration defaults

Yes, I like that!

I don't know if it could work or not.

we could replace CONF for a list of UserConfigobjects, and make them get saved in different init files:

.spyder/conf/
├── projects.ini
├── pylint.ini
...
├── editor.ini
└── variableexplorer.ini

@rlaverde I think we could save them in the same file for all bundled plugins, and on separate files for external plugins.

Yeah, that would break the update mechanism we have in place (i.e. CONF_VERSION).

I think It'll be better to add CONF_VERSION in each plugin configuration file.

Although the difficult part will be migrating between one file and configuration file for each plugin, and migrating changes from 3.x to master will be manually 😕

I think It'll be better to add CONF_VERSION in each plugin configuration file.

No, no, it's too cumbersome and problematic, as you point out correctly.

We just need to figure out how to handle this for external plugins.

I think splitting conf files could be the best approach in the long term:

PROS:

  • Same structure and logic for managing external and internal plugins
  • Less merge conflicts (in the CONF_VERSION number) when changing preferences in different plugins
  • Configurations will be independent for each plugin, if a file gets corrupted only configurations for a plugin will be lost

CONS:

  • Migration could be difficult:
  • Merge changes from 3.x

We could use the same UserConfig class, leave the same function (for backward compatibility) and change how it works internally, I could contain a dict of a PluginConfig objects, this new class will manage saving/loading from a config file (reusing some of the logic of UserConfig)

Migration: UserConfig could have a method for migrate from old configurations, It'll load values as it currently does, and then convert the list of dict, in the dict of PluginConfig

Merge 3x: We could wait until spyder3.4.4 is released, less options will be changed/added after that, and we could move issues that change options to master

We need to generalize the config settings a bit (for external plugins), but leave things as they for normal bundled ones. A plugin needs to be able to define the config file it points to, the section and the defaults. If we generalize this, we can have things as they are, internal plugins will point to the same file and the same defaults. External plugins will point to a different file and a different set of defaults.

I agree with @goanpeca. Let's not get into a messy and unneeded refactoring of our settings systems.

I agree with @goanpeca too, we could Implement that approach as I mention above, without splitting the files. I'll start working in #4306

And after testing that everything works with external plugins I'll insist in spliting settings and breaking everything 🙈

And after testing that everything works with external plugins I'll insist in splinting settings and breaking everything 🙈

You can try 😈

I'll insist in spliting settings and breaking everything

Sorry, I don't agree with this. As I told @goanpeca a long time ago, our settings system is a critical part of Spyder and it took us a long time to stabilize it, so I don't want to mess around with it again.

I tried to add PLUGIN_CLASS to internal plugins, and load them as external plugins are loaded, as I discussed with @ccordoba12 in a dev meeting.

But after some struggle I realize that It's not possible:

  • This cause several circular imports (involving syntaxhigligter, the editor, and config/gui mainly) Although them could be fixed (moving highlighter and introspection logic into the editor, and moving color_sheme logic out of config/gui.py )

  • This also broke the import order, because some imports (of inner stuff of a plugin) like these cause then module to be imported, and the __init__.py to be executed, firing practically the import of all the plugin; and this cause some unwanted behaviors (e.g. the ipyconsole fails to start)

What I think the solution could be:

  • Using a static configuration file for plugins, instead of the __init__.py (maybe a conf.ini file)
  • Using a python file, but not the __init__.py, something like conf.py for store configurations, this will prevent the import of the plugin class when importing their widgets or utils (although when importing conf.py it will fire the import of all the plugin)

Personally I prefer a static file, I will allow to load some information of the plugin without importing the plugin

Using a static configuration file for plugins, instead of the __init__.py (maybe a conf.ini file)

I don't think this is viable, we rely on being able to "execute" some of the config to be able to decide on values.

Using a python file, but not the __init__.py, something like conf.py for store configurations, this will prevent the import of the plugin class when importing their widgets or utils (although when importing conf.py it will fire the import of all the plugin)

This is more viable I would say

I don't think this is viable, we rely on being able to "execute" some of the config to be able to decide on values.

Hmm, no the preferences of the plugin, but the PLUGIN_CLASS and the other configurations/information that we add to the plugin

Hmm, no the preferences of the plugin, but the PLUGIN_CLASS and the other configurations/information that we add to the plugin

ah ok, then conf.py is a bad name.

A bit more than one year after @rlaverde started this big refactoring, this is finally ready!!

Gee, I hope to not do it again!!

@ccordoba12 Famous last words...

I think this is the last major refactoring we will do. I don't see another one like this coming in the horizon.

That's the thing about refactoring, isn't it...you don't see them coming, until it's too late jeje...

But in all seriousness, you're older and wiser and don't make a tenth the mistakes I do, so I'm sure you're right—let's hope so, anyways!

Was this page helpful?
0 / 5 - 0 ratings