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

PR: Refactor Spyder configuration system to use an observer pattern #14852

Merged
merged 76 commits into from
Mar 16, 2021

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Mar 2, 2021

Description of Changes

This PR enables the Spyder configuration system to use an observer pattern in order to propagate changes across all the application actions/options/etc. This PR will supersede the current API configuration propagation logic

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @andfoy

@andfoy andfoy added this to the v5.0alpha6 milestone Mar 2, 2021
@andfoy andfoy self-assigned this Mar 2, 2021
@jitseniesen
Copy link
Member

Thanks on tackling this area because I found the old API hard to work with. Where is the new design explained? I am lazy and don't want to search through 100+ commits and files changed. 😄

@andfoy
Copy link
Member Author

andfoy commented Mar 3, 2021

@jitseniesen Basically the idea is to register any object that requires updates on a configuration option of a section via CONF.observe_configuration(observer, section, option). If the option is None, the object is notified on any change on the section. Likewise, since now we support tuple options to access nested dictionary keys, an object can subscribe to a partial tuple key. A registered object will be notified via the on_configuration_change(option, section, value) method each time a registered option/section changes via CONF.set.

This PR also introduces a decorator spyder.api.decorators.on_conf_change to register automatically methods that receive changes on specific keys, e.g.,

from spyder.config.manager import CONF
from spyder.api.decorators import on_conf_change

class ConfObserver:
    def __init__(self):
        # Register class to listen for changes in all the registered options
        for section in self._configuration_listeners:
            observed_options = self._configuration_listeners[section]
            for option in observed_options:
                CONF.observe_configuration(self, section, option)

    def on_configuration_change(self, option, section, value):
        section_recievers = self._configuration_listeners.get(section, {})
        option_recievers = section_recievers.get(option, [])
        for receiver in option_recievers:
            method = getattr(self, receiver)
            method(value)

    @on_conf_change(section='editor')
    def section_change_reciever(self, section):
          # section contains the new values of the editor section
          ...
    
    @on_conf_change(section='main_interpreter', option='custom_interpreter')
    def on_interpreter_change(self, interpreter):
          # interpreter contains the value of main_interpreter
          ...

Furthermore, to simplify things, this PR adds a new class mixin spyder.api.mixins.SpyderConfigurationObserver that already implements the aforementioned protocol. By inheriting from this mixin, the previous example can be simplified to

from spyder.api.mixins import SpyderConfigurationObserver

class ConfObserver(SpyderConfigurationObserver):
    @on_conf_change
    def this_section_change(self, value):
          # Listen for all changes in the widget `CONF_SECTION`
          # value contains the section options as a dictionary.
          ...

    @on_conf_change(section='editor')
    def section_change_reciever(self, section):
          # section contains the new values of the editor section
          ...

    @on_conf_change(option='opt')
    def on_opt_change(self, value):
          # value contains the new value of `opt` in the widget `CONF_SECTION`.
          ...

    @on_conf_change(option=['opt1', 'opt2', 'opt3'])
    def on_opts_change(self, option, value):
          # Receive multiple option change. Option will contain the option name and value, its corresponding value.
         ...

    @on_conf_change(section='section', option=['opt2', 'opt3', 'opt4', ('opt', 'nested_dict', 'value')])
    def on_section_multiple_option_change(self, option, value):
          # Recieve multiple option change from section `section`.
          ...
    
    @on_conf_change(section='main_interpreter', option='custom_interpreter')
    def on_interpreter_change(self, interpreter):
          # interpreter contains the value of main_interpreter
          ...

By default, SpyderPluginV2 and SpyderWidgetMixin classes will inherit from SpyderConfigurationObserver, making all of the new API plugins compatible with this new design. Also, all SpyderWidgetMixin will inherit its parent CONF_SECTION value, thus enabling on_conf_change to omit the section parameter to fallback to the value of CONF_SECTION.

FInally, all new actions created using self.create_action of the new API now can specify a section and option that is read and written to, thus they are also subject to the new observer pattern, where they will change their state if a change is done on the configuration option observed.

I hope this explanation is clear enough, otherwise, feel free to ask any question

@pep8speaks
Copy link

pep8speaks commented Mar 6, 2021

Hello @andfoy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 26:80: E501 line too long (80 > 79 characters)

Line 340:13: E731 do not assign a lambda expression, use a def

Line 658:1: E302 expected 2 blank lines, found 1

Line 57:25: E127 continuation line over-indented for visual indent

Comment last updated at 2021-03-16 00:19:43 UTC

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @andfoy! I left an initial review for you.

spyder/api/decorators.py Outdated Show resolved Hide resolved
spyder/api/mixins.py Outdated Show resolved Hide resolved
spyder/api/widgets/mixins.py Show resolved Hide resolved
spyder/app/tests/test_mainwindow.py Outdated Show resolved Hide resolved
spyder/config/types.py Outdated Show resolved Hide resolved
spyder/plugins/findinfiles/tests/test_widgets.py Outdated Show resolved Hide resolved
spyder/plugins/findinfiles/tests/test_widgets.py Outdated Show resolved Hide resolved
spyder/plugins/findinfiles/tests/test_widgets.py Outdated Show resolved Hide resolved
spyder/plugins/findinfiles/tests/test_widgets.py Outdated Show resolved Hide resolved
spyder/utils/qthelpers.py Show resolved Hide resolved
@andfoy andfoy marked this pull request as ready for review March 15, 2021 23:07
@andfoy andfoy changed the title [WIP] PR: Refactor Spyder configuration propagation to use an observer pattern. PR: Refactor Spyder configuration propagation to use an observer pattern. Mar 15, 2021
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

@andfoy, this is a more in-depth review for you.

spyder/api/decorators.py Outdated Show resolved Hide resolved
spyder/api/mixins.py Outdated Show resolved Hide resolved
spyder/api/mixins.py Outdated Show resolved Hide resolved
spyder/api/mixins.py Outdated Show resolved Hide resolved
spyder/api/mixins.py Outdated Show resolved Hide resolved
spyder/plugins/completion/api.py Show resolved Hide resolved
spyder/plugins/completion/api.py Outdated Show resolved Hide resolved
spyder/plugins/help/widgets.py Outdated Show resolved Hide resolved
spyder/plugins/help/widgets.py Outdated Show resolved Hide resolved
spyder/utils/qthelpers.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 changed the title PR: Refactor Spyder configuration propagation to use an observer pattern. PR: Refactor Spyder configuration system to use an observer pattern Mar 16, 2021
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @andfoy! Fantastic job with this refactoring!!

@ccordoba12 ccordoba12 merged commit 0244c76 into spyder-ide:master Mar 16, 2021
@andfoy andfoy deleted the conf_refactor branch March 16, 2021 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants