pymmcore-plus / useq-schema

An implementation agnostic schema for describing a sequence of events during a multi-dimensional imaging acquisition.
https://pymmcore-plus.github.io/useq-schema/
BSD 3-Clause "New" or "Revised" License
15 stars 5 forks source link

fix: fix z_pos when there is no z_plan #42

Closed fdrgsp closed 2 years ago

fdrgsp commented 2 years ago

At the moment, when there is no z_plan (NoZ), when we set the stage to move and acquire images at different xy positions (e.g. through the napari-micromanager mda position table), the z_pos is always None even if it is defined in the experiment.

This PR fix this problem in the iter_events methods of the MDASequence class.

codecov[bot] commented 2 years ago

Codecov Report

Merging #42 (1b80cb2) into main (15f16bf) will increase coverage by 0.41%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   85.40%   85.81%   +0.41%     
==========================================
  Files           8        8              
  Lines         411      409       -2     
==========================================
  Hits          351      351              
+ Misses         60       58       -2     
Impacted Files Coverage Δ
useq/_mda_sequence.py 81.45% <0.00%> (+1.29%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

fdrgsp commented 2 years ago

is this ready to go? Is there a test you could add that fails on main for

yes it is ready to go. There is no error (it works), it is only not considering the z position if there is no z_plan. Maybe I can add a test in pymmcore-plus after this.

tlambert03 commented 2 years ago

is this ready to go? Is there a test you could add that fails on main for

yes it is ready to go. There is no error (it works), it is only not considering the z position if there is no z_plan.

well ... you said:

when we set the stage to move and acquire images at different xy positions (e.g. through the napari-micromanager mda position table), the z_pos is always None even if it is defined in the experiment.

and then you opened this PR to fix it ... so, looking at the code you submitted, it looks to me like creating a MDASequence and then calling iter_events() should have different behavior now than before right? that's what I mean... can you add a test for that?

(i.e. any code that needs to be modified in this repository should be accompanied by tests in this repository)

fdrgsp commented 2 years ago

@tlambert03

you mean something simple like this?

def test_z_position() -> None:
    mda = MDASequence(
        axis_order="tpcz",
        stage_positions=[(222, 1, 10), (111, 1, 20)]
    )
    assert not mda.z_plan
    for event in mda:
        assert event.z_pos
tlambert03 commented 2 years ago

yep, exactly. Something that fails on the main branch, but is fixed here in your PR. something to make sure that whatever you've done here won't be "lost" accidentally in the future. tests can be super simple