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

Add flags to control the start of the application #200

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

olivierdelree
Copy link
Contributor

Closes #199.

It is currently impossible to get a visiomode.core.Visiomode object without incurring the cost of starting the whole application (i.e., starting the webpanel and getting stuck in the main application loop).
This PR puts such actions behind their own flags, meaning it is possible to initialise a minimal version of the application.

Another aspect the PR addresses is the import of plugins. They (visiomode.devices, visiomode.protocols, visiomode.stimuli) are imported automatically when importing visiomode (or any submodules). And because of a visiomode.plugins.load_modules_dir() at the end of said plugins, they are loaded at import time. The refactor proposed here moves their initialisation into the main app initialisation and puts it behind a flag.
This could be a problem if they are required to be initialised under other circumstances but I have not been able to produce an error when playing around with the application. Do let me know if there's a use case that would require them to be loaded outside of the main application startup.

Currently, importing `visiomode.core` leads to automatically loading all
the plugins (i.e., `devices`, `protocols`, and `stimuli`). Although this
makes sense in the context of the full application and is the expected
use case for the end user, this is a bit cumbersome when trying to
create minimal tests.
To follow the spirit of the previous commit, putting plugin loading
behind a flag in the main application initialisation makes it possible
to import the various modules without incurring the burden of loading
the plugins.

Also note that importing `visiomode.core` is not really a choice as
`visiomode.__init__` itself imports `visiomode.core`, making it a
requirement to load the plugins regardless of what imports are done
by the user. This is especially cumbersome when developing tests for
parts of the application that do not make use of the plugins (e.g.,
`visiomode.config`) but which are forced to incur the import cost.
@olivierdelree olivierdelree self-assigned this May 27, 2024
@olivierdelree olivierdelree added the enhancement New feature or request label May 27, 2024
@celefthe celefthe self-requested a review June 10, 2024 14:23
@celefthe
Copy link
Member

This could be a problem if they are required to be initialised under other circumstances but I have not been able to produce an error when playing around with the application. Do let me know if there's a use case that would require them to be loaded outside of the main application startup.

I can't think of a good use case for loading plugins after startup; I think if a user uploads a plugin it's safer to just instruct them to restart the app.

Copy link
Member

@celefthe celefthe left a comment

Choose a reason for hiding this comment

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

As discussed in person, it would be nicer to refactor webpanel and application loop initialisation out of __init__ and into a run() method to decouple object instantiation from a bunch of windows and webservers popping up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to create app object without running whole application
2 participants