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

npe2 cookiecutter notes #69

Open
5 of 7 tasks
tlambert03 opened this issue Dec 13, 2021 · 2 comments
Open
5 of 7 tasks

npe2 cookiecutter notes #69

tlambert03 opened this issue Dec 13, 2021 · 2 comments

Comments

@tlambert03
Copy link
Member

tlambert03 commented Dec 13, 2021

hey @nclack

Taking a look through the npe2 cookiecutter, instead of a PR, just want to put some tasks here that we can tackle individually:

  • we shouldn't use from numpy.typing import ArrayLike in _writer for two reasons: first, it's only available in pretty new versions of numpy (I think 1.20+) and we didn't pin numpy in the users' setup.cfg. Second, it's not an accurate type., layer data can come in many types. We're trying to codify this with the LayerDataProtocol, but i wouldn't use that either. Just call it typing.Any for now.
  • writers need more docstrings with clearer expectations about what they expect, and what they should do. I know there's a link there, but it would be nice if the cookiecutter stood alone a bit more.
  • writer type signatures are wrong #71
    def write_single_image(path: str, data: Any): ...
    # should be changed to
    def write_single_image(path: str, data: Any, meta: dict): ...
    and
    def write_multiple(path: str, data: List[ArrayLike], layer_types: List[str]): ...
    # should be change to
    def write_multiple(path: str, List[FullLayerData]): ...
    The args are passed here
  • ... obviously, we should make the equivalent of the hook_specifications.py for npe2, so we have one authoritative place to look for all signatures expected by npe2. (See Start to make command APIs clearer npe2#61)
  • The python_name for command napari-foobar.writer is duplicated in the commands list #70
      commands:
        - id: napari-foobar.get_reader
          python_name: napari_foobar._reader:napari_get_reader
          title: Open data with napari FooBar
        - id: napari-foobar.writer
          python_name: napari_foobar._reader:napari_get_reader
          title: Save data with napari FooBar
  • (This one is in main like this, but just noting this so I don't forget)... in _widget.py, we should prefer viewer: 'napari.viewer.Viewer'over using the param named napari_viewer, and we should add at the top:
if TYPE_CHECKING:
    import napari.viewer
    import napari.layers
  • writer tests need improvement (possible others as well)
@nclack
Copy link
Collaborator

nclack commented Dec 13, 2021

the equivalent of the hook_specifications.py for npe2

I like it a lot

@nclack
Copy link
Collaborator

nclack commented Dec 13, 2021

the writer test is anemic/not-present which contributes to some of these. I let that slip my mind. I'll edit the issue to add that to the list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants