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

remove napari pin and npe2 support #169

Merged
merged 1 commit into from Oct 13, 2022
Merged

Conversation

haesleinhuepf
Copy link
Contributor

Hi Johannes @jo-mueller ,

this PR should make it possible to run napari-stress in napari > 0.4.15 AND have its menu entries in the tools menu. I've tested this in an empty conda environment like this:

mamba create --name napari_only python=3.9 napari=0.4.16 -c conda-forge
conda activate napari_only
cd napari-stress
pip install -e .

Afterwards, napari-stress shows up in the tools menu, but not in the plugins menu. If we want to have it in the plugins-menu as well, we would need to implement something like this to be fully npe1 compatible.

I would not do it. I think its enough to have it in the Tools menu only.

Let me know what you think!

Best,
Robert

@jo-mueller
Copy link
Member

jo-mueller commented Oct 3, 2022

@haesleinhuepf thanks for looking into this, can confirm that it works. One thing I'm missing though (should I send a PR to this one?) is that while transferring functions from the plugins menu to the tools menu, it also removes provided sample data from the File > Open Sample >... dialogue. I would have to thoroughly annotate some more functions here.

Edit: Maybe a separate Tools menu entry Sample Data would be nice?

I would also like to investigate first how this integrates with workflows. I think the workflow functionality supersedes the frame_by_frame functionality in napari-stress. Which I don't mind, but there is for instance the stress analysis toolbox that handles 4D pointclouds and returns a List[LayerDataTuple] - if the workflow slices the 4D pointcloud into single timeframes before the toolbox even gets to see the data.

Before going on and merging this I would first like to figure out how to

  • have workflows handle arbitrary data types (Possible related PR?)
  • aggregate data at the end of workflows into 4D data (see this related PR)

As for the failing tests - it seems like qt is missing during the github CI tests - maybe it could help to simply add pyqt as a dependency for test runtime? I think this could be done as here

@haesleinhuepf
Copy link
Contributor Author

haesleinhuepf commented Oct 3, 2022

@haesleinhuepf thanks for looking into this, can confirm that it works. One thing I'm missing though (should I send a PR to this one?) is that while transferring functions from the plugins menu to the tools menu, it also removes provided sample data from the File > Open Sample >... dialogue. I would have to thoroughly annotate some more functions here.

Edit: Maybe a separate Tools menu entry Sample Data would be nice?

Good catch! I'm against a separate entry in the Tools menu. File > Open samples is supported by npe1. Example here:
https://github.com/clEsperanto/napari_pyclesperanto_assistant/blob/master/napari_pyclesperanto_assistant/_napari_plugin.py#L52-L66

Would you mind taking care of this? Thanks!

Before going on and merging this I would first like to figure out how to

  • have workflows handle arbitrary data types (Possible related PR?)
  • aggregate data at the end of workflows into 4D data (see this related PR)

That makes sense, but is unrelated with napari-version / npe compatibility in this PR. I propose to handle this in a separate PR. Btw, the linked "PR" are no PRs.

As for the failing tests - it seems like qt is missing during the github CI tests - maybe it could help to simply add pyqt as a dependency for test runtime? I think this could be done as here

I was replacing the github-workflows in a couple of projects recently to fix tests there. Examples:

Thanks!

@jo-mueller
Copy link
Member

Alright - I updated some of the descriptions in the docs, added a few register_dock_widgets() in a different branch so this can go through. Thanks for the contribution! @haesleinhuepf

@jo-mueller jo-mueller merged commit d936039 into main Oct 13, 2022
@haesleinhuepf haesleinhuepf deleted the enable_higher_napari_versions branch October 14, 2022 05:28
@jo-mueller jo-mueller mentioned this pull request Oct 25, 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.

None yet

2 participants