Skip to content

Add plugin discovery#22

Open
clane9 wants to merge 2 commits into
mainfrom
enh/plugins
Open

Add plugin discovery#22
clane9 wants to merge 2 commits into
mainfrom
enh/plugins

Conversation

@clane9
Copy link
Copy Markdown
Contributor

@clane9 clane9 commented Aug 21, 2024

We need some way to discover user defined figures outside of the main niftyone namespace. This adds a very basic plugin discovery based on the naming convention niftyone_{plugin_name}. See the python packaging guide.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 21, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (b3f7cb3) to head (95c0715).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #22   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files          18       19    +1     
  Lines         726      730    +4     
=======================================
+ Hits          724      728    +4     
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kaitj
Copy link
Copy Markdown
Contributor

kaitj commented Sep 25, 2024

Getting a chance to take a closer look at this now - this looks good to me. If I am understanding how it works, the plugin would be installed in the environment / part of the PYTHONPATH, prefixed with niftyone_ so that importlib can find it. More of a note for now, but we should make note of the naming convention in the documentation.

Theoretically, the plugin can be anything, but taking the DWI figure generation as an example - if it were to be a plugin, it'd be a package named niftyone_dwi, where actual creation of the figure could be part of the "ViewGenerator" class. I think as part of the intialization, we could check if it is an instance of the "ViewGenerator" class and call the register decorator on it if it is. The current generators (

from .figures.func import CarpetPlotGenerator, MeanStdGenerator
from .figures.multi_view import (
SliceVideoGenerator,
ThreeViewGenerator,
ThreeViewVideoGenerator,
)
) all do this from the function call themselves, so we could do this after the existing generators are registered or are we wanting the plugins to call the register decarator themselves (similar to the existing ones?)

@clane9
Copy link
Copy Markdown
Contributor Author

clane9 commented Sep 26, 2024

My thought was that the plugin code would mirror the internal code, so that it would be the plugin code's responsibility to inherit from the relevant base classes and then register. The benefit is that we don't need to do anything other than just import the plugin modules.

Ofc we will want to document this, but I think the documentation can wait until things are a bit more crystallized.

@kaitj
Copy link
Copy Markdown
Contributor

kaitj commented Sep 26, 2024

Ahh that does make sense. I think I changed this in the PR that merges into this one, but that is an easy enough change to switch it back.

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.

2 participants