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_hook_implementation decorator #36

Merged
merged 6 commits into from
Jun 14, 2024
Merged

Conversation

jni
Copy link
Member

@jni jni commented Dec 11, 2023

  • Remove a remnant napari_hook_implementation from get_writer
  • Update python versions in setup.cfg

Closes #31

Czaki
Czaki previously approved these changes Dec 11, 2023
@Czaki Czaki dismissed their stale review December 11, 2023 09:20

please fix test matrix

@jni
Copy link
Member Author

jni commented Dec 11, 2023

Thanks @Czaki! I've gone with the strategy of only testing earliest and latest python.

@jni
Copy link
Member Author

jni commented Mar 24, 2024

@Czaki I dropped this. Just found it again by going through my open PRs. Can you please sanity-check it before I merge? 🙏

Copy link
Member

@andy-sweet andy-sweet left a comment

Choose a reason for hiding this comment

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

Seems fine to me. What's the motivation behind not testing the Python versions in the middle? Avoiding overkill? The tests seem to run pretty quickly, so doesn't seem to necessary, but I also don't oppose.

@jni
Copy link
Member Author

jni commented Jun 14, 2024

yeah we do that in the napari repo I think? Anyway, someone at some point suggested that this was a good strategy and I agree. It seems exceedingly unlikely that code would run in .9 and .12 but not in the middle...

@jni jni merged commit 5778489 into napari:master Jun 14, 2024
7 checks passed
@jni jni deleted the remove-hook branch June 14, 2024 00:42
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.

Migrate out of npe1 completely
3 participants