Always add Empty context key, even if action is already registered
#7106
Conversation
|
Looking into the test failure on 3.9, could be legit. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7106 +/- ##
==========================================
- Coverage 92.96% 92.86% -0.10%
==========================================
Files 618 619 +1
Lines 56531 56550 +19
==========================================
- Hits 52552 52515 -37
- Misses 3979 4035 +56 ☔ View full report in Codecov by Sentry. |
|
Same min req test if failing in a different PR: |
Music to my ears 😆 |
|
I don't get it, cause the constraints should protect us from stuff like this. Nothing should have changed from yesterday. |
Yeah I don't understand what would've changed - I guess a package that has no upper version constraint? But I don't even know where to go to look at the constraints we've set for |
|
|
|
So looking more at the failure, it looks like File "/home/runner/work/napari/napari/.tox/py39-linux-pyqt5-cov/lib/python3.9/site-packages/setuptools/_vendor/typeguard/_checkers.py", line 71, in <module>
from typing_extensions import Any as SubclassableAny
ImportError: cannot import name 'Any' from 'typing_extensions' (/home/runner/work/napari/napari/.tox/py39-linux-pyqt5-cov/lib/python3.9/site-packages/typing_extensions.py) |
|
Here's the constraints file, but it's empty: |
|
I opened an issue for the
Yep, this confuses me too and I don't understand where we get the info from - see this zulip thread where @Czaki tells me how to run |
|
Oh, it's cause we use a tox plugin: |
References and relevant issues
Closes #7105
Description
This PR ensures that even if the dummy
Emptyaction is registered with theapp, we still assign itscontext_key. I checked that there's no danger in assigning the context key multiple times (like there is in registering an action multiple times), so this shouldn't have any undesirable side effects... 😬Because new viewers in notebooks share the same
appinstance, theEmptydummy action is already registered with all launched viewers, but each launched viewer gets its own instance of theLayerListand therefore its own new instance of thecontext. This means we keep the registered actions, but have to reassign all the context keys. I think this is terrible, but should be fixed in concert with whatever we decide in #7101.