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

feature/convert-to-npe2 #49

Merged
merged 4 commits into from
Jul 21, 2022
Merged

Conversation

tlambert03
Copy link
Contributor

@tlambert03 tlambert03 commented Jul 21, 2022

converts plugin to npe2 (docs on creating npe2 plugins)

As discussed on zulip, the biggest impact for this plugin is the inability to have two virtual plugin entry points (in/out of mem) within a single plugin package. while we can provide multiple readers in a single plugin, we discussed that it might be better to just choose based on the filesize/memory available. I did that here with a 30% available mem or 4GB file size threshold. Seems to work well in local tests

this also:

  • changes print statements to logs
  • adds an npe2-specific test
  • delays a few imports, or moves them entirely to TYPE_CHECKING clauses

@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2022

Codecov Report

Merging #49 (7ad6c1d) into main (728bf8f) will increase coverage by 5.17%.
The diff coverage is 67.56%.

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   83.59%   88.77%   +5.17%     
==========================================
  Files           7        3       -4     
  Lines         189      187       -2     
==========================================
+ Hits          158      166       +8     
+ Misses         31       21      -10     
Impacted Files Coverage Δ
napari_aicsimageio/tests/test_core.py 94.44% <66.66%> (-5.56%) ⬇️
napari_aicsimageio/core.py 85.93% <67.85%> (-0.15%) ⬇️
napari_aicsimageio/__init__.py
napari_aicsimageio/tests/__init__.py

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 728bf8f...7ad6c1d. Read the comment docs.

Copy link
Collaborator

@evamaxfield evamaxfield left a comment

Choose a reason for hiding this comment

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

A single comment otherwise looks good to me!

python_name: napari_aicsimageio.core:get_reader
readers:
- command: napari-aicsimageio.get_reader
filename_patterns: [
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is rough but understandable. Should become easier to generate in aicsimageio v5 though because plugins report which filetypes they support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not strictly necessary... we could also use filename_patterns: ["*"] ... but this is preferable, as it will allow napari to do things like this make suggestions to a user that doesn't have aicsimageio installed that "there's a plugin that can help with this"

@evamaxfield
Copy link
Collaborator

Wait even my comment is ignorable. Merging now!

@evamaxfield evamaxfield merged commit ea06866 into AllenCellModeling:main Jul 21, 2022
@tlambert03 tlambert03 deleted the npe2 branch July 21, 2022 20:31
@evamaxfield evamaxfield mentioned this pull request Jul 21, 2022
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.

3 participants