-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add a new SequenceAcquisition event signals #115
Conversation
Codecov Report
@@ Coverage Diff @@
## main #115 +/- ##
==========================================
+ Coverage 88.45% 88.68% +0.22%
==========================================
Files 33 33
Lines 1447 1476 +29
==========================================
+ Hits 1280 1309 +29
Misses 167 167
Continue to review full report at Codecov.
|
oooh actually what do you think about adding this directly to MMCore? I'lll ask over there as well. |
See micro-manager/mmCoreAndDevices#179 - even if they do get added there we will need to make some changes here. Having looked at that little more carefully I also think we probably can't get away with just one signal here. I think we need both a started and stopped signal are necessary as we should have the parameters of the startedsequence emitted in the started signal. We also need to be careful here to maintain the typing overloads on startSequenceAcquistion: which should also get re-implemented here to as they will emit different signals than the |
I agree with all of @ianhi's comments. Also think we can add this before any upstream changes are made... (but, it would be ideal if we could somehow anticipate the exact event/callback names that might be used upstream so our API wouldn't change). I also agree about the typing of the methods, and also think that we should just use the same method names (rather than |
plus... tests :) |
for more information, see https://pre-commit.ci
…into new_signals
for more information, see https://pre-commit.ci
…into new_signals
for more information, see https://pre-commit.ci
…into new_signals
Co-authored-by: Ian Hunt-Isaak <ianhuntisaak@gmail.com>
…into new_signals
for more information, see https://pre-commit.ci
This PR adds three new
SequenceAcquisition
event signals:startContinuousSequenceAcquisition = Signal()
# when continuousSequenceAcquisition is startedstartSequenceAcquisition = Signal(str, int, float, bool)
# when SequenceAcquisition is startedstopSequenceAcquisition = Signal(str)
# when (Continuous)SequenceAcquisition is stoppedand three methods to the core to implement the above signals:
startContinuousSequenceAcquisition()
startSequenceAcquisition()
stopSequenceAcquisition()
linked to PR https://github.com/tlambert03/napari-micromanager/pull/148