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 npe1_shim field to schema #182

Merged
merged 5 commits into from
Jun 19, 2022
Merged

Conversation

tlambert03
Copy link
Collaborator

@tlambert03 tlambert03 commented Jun 19, 2022

This adds a new top level npe1_shim field to the schema, which will make it possible to determine whether a serialized manifest represents a shimmed npe1 plugin or not.

This also makes sure that all shim manifests have their contributions loaded before serialization

cc @DragaDoncila

@tlambert03 tlambert03 requested a review from DragaDoncila June 19, 2022 15:18
@codecov
Copy link

codecov bot commented Jun 19, 2022

Codecov Report

Merging #182 (032cd9e) into main (772f49f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #182   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines         1789      1794    +5     
=========================================
+ Hits          1789      1794    +5     
Impacted Files Coverage Δ
npe2/manifest/_npe1_adapter.py 100.00% <100.00%> (ø)
npe2/manifest/schema.py 100.00% <100.00%> (ø)

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 772f49f...032cd9e. Read the comment docs.

Copy link
Contributor

@DragaDoncila DragaDoncila left a comment

Choose a reason for hiding this comment

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

This looks great to me, thank you @tlambert03 🙏 . Two questions:

  • Could we use this in the plugin dialog on the napari side to avoid npev = 'shim' if 'npe1' in type(manifest).__name__.lower() else 2? I know with napari #4692 it'll matter less but it may still be useful for users to know whether a plugin is native npe2 or not.
  • Now that we load contributions before serializing, does that mean we can get rid of the call to index_npe1_adapters on the napari hub side which we were using to force contribution discovery, even though we're exporting to json? I assume _serialized_data gets called when writing to any filetype?

@tlambert03
Copy link
Collaborator Author

Could we use this in the plugin dialog on the napari side to avoid npev = 'shim' if 'npe1' in

yes!

does that mean we can get rid of the call to index_npe1_adapters on the napari hub side which we were using to force contribution discovery, even though we're exporting to json

yes!

@tlambert03
Copy link
Collaborator Author

tlambert03 commented Jun 19, 2022

does that mean we can get rid of the call to index_npe1_adapters on the napari hub side which we were using to force contribution discovery, even though we're exporting to json

actually, even better @DragaDoncila ... you should just be able to do this now from the cli

npe2 parse napari-compressed-labels-io > napari.yaml
name: napari-compressed-labels-io
schema_version: 0.1.0
contributions:
  commands:
  - id: napari-compressed-labels-io.get_reader
    title: Get Reader
    python_name: napari_compressed_labels_io._reader:get_label_image_stack
  - id: napari-compressed-labels-io.get_reader
    title: Get Reader
    python_name: napari_compressed_labels_io._reader:get_zarr_labels
  - id: napari-compressed-labels-io.write_labels
    title: Write Labels
    python_name: napari_compressed_labels_io._writer:labels_to_zarr
  readers:
  - command: napari-compressed-labels-io.get_reader
    filename_patterns:
    - '*'
    accepts_directories: true
  - command: napari-compressed-labels-io.get_reader
    filename_patterns:
    - '*.zarr'
    accepts_directories: true
  writers:
  - command: napari-compressed-labels-io.write_labels
    layer_types:
    - labels
    filename_extensions: []
    display_name: labels
npe1_shim: true

I'll add something to allow export to json

@tlambert03 tlambert03 merged commit dce3252 into napari:main Jun 19, 2022
@tlambert03 tlambert03 deleted the add-shim-field branch June 19, 2022 19:36
@tlambert03 tlambert03 added the enhancement New feature or request label Jun 21, 2022
@tlambert03 tlambert03 changed the title Add npe1_shim field to schema Add npe1_shim field to schema Jun 21, 2022
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.

2 participants