Add some old async definitions back #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
All of this looks great. I think it covers everything we talked about. Are you ok with going ahead and merging in with my branch? I can't figure out why CI isnt running or how to force trigger it. If CI fails once it gets merged back into my branch I can fix it up. Unless we decide to go with alternative nuclear PR instead... |
Yep. There's probably a couple of things to clean up, but we can do that on your branch. Then open the PR on napari/napari. I think even if we go for a bigger removal, we may still need some of the deprecation, so I think some of these changes are still useful. |
kcpevey
pushed a commit
that referenced
this pull request
Apr 6, 2023
* Add back some things [skip ci] * Remove pytest markers for old async * Deprecate loaded * Empty commit to rerun CI * Empty commit to rerun CI * Ignore deprecated introspection
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes represent most of the changes we settled on after code pairing. Haven't thought all of these through yet, but wanted to share and also run CI to see what breaks.
Overall, I am starting to feel like just removing everything old async/octree is the way to go since, like you said (and now I've felt first-hand), there is some complicated nuance here. And I'm not convinced that any discoverability of the old code being in the repo is worth it - actually I might argue that removing everything at once makes things easier to understand retrospectively too.