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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Functional plugins infrastructure #263

Closed
wants to merge 11 commits into from
Closed

[WIP] Functional plugins infrastructure #263

wants to merge 11 commits into from

Conversation

@jni
Copy link
Member

@jni jni commented May 16, 2019

Description

This is a skeleton for what I envision functional plugins to look like, as a first pass. Things like live update etc I think we should leave till later.

For reference, here's what happens when I crawl the test_validation module from image-types:

In [6]: plug.crawl(tv)                                                    
Out[6]: {'gaussian_filter': <function test_validation.gaussian_filter(image: 'Image', sigma: 'Sigma') -> 'Image'>}

As a side bonus this PR includes a menu bar and a close-window shortcut. 馃帀

This isn't even functional yet but wanted to submit to discuss during meeting.

Q: wasn't sure whether this should live in the window or the viewer, guidance here would be much appreciated. I kinda think the window.

Type of change

  • Bug-fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

References

How has this been tested?

  • example: the test suite for my feature covers cases x, y, and z
  • example: all tests pass with my change

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
@pep8speaks
Copy link

@pep8speaks pep8speaks commented May 16, 2019

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

Line 30:21: W292 no newline at end of file

@sofroniewn
Copy link
Contributor

@sofroniewn sofroniewn commented May 16, 2019

@jni this is really cool to see - definitely helps me understand how you're thinking about all this.

I'd say this should all live on Window for now, and that at some point in time we might want to pull it out of this repo entirely and into the napari-app repo. Having it on Window will make that transition easier. I don't think Viewer should know about plugins at all.

Looking forward to more discussion later!

@sofroniewn sofroniewn added this to In progress in App, CLI, and Plugin architecture via automation May 16, 2019
@jni
Copy link
Member Author

@jni jni commented Sep 17, 2019

@sofroniewn I've picked this up again finally, fyi. It's looking pretty cool right now. Just need to hook up actions to the functions. =) I've temporarily given up on typing and we'll just have to rely on users to give sane inputs to the functions, which is The Python Way anyway. It should be easy to add typing after an initial version is working.

The next stage is to have a popup menu for each action that lets you select either existing layers or variables in the global workspace or Python literals as input. Again, this can be replaced with a drawer with e.g. preview mode in the future.

Just keeping everyone updated... ;)

@jni
Copy link
Member Author

@jni jni commented Sep 17, 2019

btw, I do get this warning when adding the scikit-image module: "The gui lock can only be acquired by one toolkit per session. The lock is already acquired by qt". I presume it has something to do with adding menu items at runtime, but I have no idea... If anyone knows what it's about, pointers are appreciated!

@codecov
Copy link

@codecov codecov bot commented Sep 17, 2019

Codecov Report

Merging #263 into master will decrease coverage by 0.24%.
The diff coverage is 31.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
- Coverage   82.01%   81.76%   -0.25%     
==========================================
  Files         136      137       +1     
  Lines       14814    14886      +72     
==========================================
+ Hits        12149    12172      +23     
- Misses       2665     2714      +49
Impacted Files Coverage 螖
napari/plugins/plugin_engine.py 22.72% <22.72%> (酶)
napari/_qt/qt_main_window.py 83.33% <46.42%> (-9.94%) 猬囷笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update bc92411...e43f1fa. Read the comment docs.

@sofroniewn
Copy link
Contributor

@sofroniewn sofroniewn commented Sep 18, 2019

@jni this looks so cool, great that you're picking it up again! i think this is a great time for us to make a push for the plugins :-)

when ready I'm very happy to reimplement the project @marshuang80 did this summer around image segmentation as a plugin - https://github.com/transformify-plugins/segmentify - right now it uses napari as a dependency and subclasses the Viewer.

We'll have to think about some additional things like how is a plugin allowed to define custom keybindings, and eventually how is a plugin allowed to request UI elements.

@jni
Copy link
Member Author

@jni jni commented Sep 18, 2019

@sofroniewn indeed! To clarify, my goal for this PR is to only provide support for "functional" plugins (things that take in layer data and spit out other layer data). I feel like other stuff can come in later PRs.

I haven't thought very much about interactive plugins, but one idea is to have them be Python functions that take the viewer as input. Then the world is their oyster. =)

Something to discuss once this is ready for review. Hold please. =)

@jni
Copy link
Member Author

@jni jni commented Sep 23, 2019

馃槀 I think we are going to have to curate a NumPy plugin rather than recommend that people add NumPy directly (note that the list below is in alphabetical order):

Screen Shot 2019-09-23 at 4 29 12 pm

@sofroniewn sofroniewn changed the title WIP (Very IP): Functional plugins infrastructure [WIP] Functional plugins infrastructure Oct 16, 2019
@sofroniewn sofroniewn mentioned this pull request Nov 8, 2019
@goanpeca
Copy link
Contributor

@goanpeca goanpeca commented May 18, 2021

Is this something we can close now @jni? Thanks!

@sofroniewn sofroniewn added this to the 0.2 milestone May 20, 2021
@sofroniewn
Copy link
Contributor

@sofroniewn sofroniewn commented May 31, 2021

ToDo here is to convert this into an issue containing the code we want to preserve/ reuse in the future - cc @jni

@sofroniewn sofroniewn removed this from the 0.2 milestone May 31, 2021
@sofroniewn sofroniewn added this to the 0.4 milestone May 31, 2021
@jni jni added the make-issue label Oct 25, 2021
@jni
Copy link
Member Author

@jni jni commented Oct 27, 2021

Closing as outdated, discussion to continue in #3525.

@jni jni closed this Oct 27, 2021
App, CLI, and Plugin architecture automation moved this from In progress to Done Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants