Repository: bluesky/bluesky (python)

Issue:
Remove or Disable-by-Default Pre-Declare
Reports have come in from a number of sources of bugs rising as a result of pre-declare (https://github.com/bluesky/bluesky/pull/1542)

See also: https://github.com/bluesky/bluesky/issues/1594

I think the easiest solution for now is to disable it in the built-in plans but leave it in bluesky for those who want it. It may become the default behaviour in future when its issues are addressed.

Pull request:
API: add ENV to opt-in to automatic stream predeclaring
<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->

This is the code + testing part of making stream pre-declaration opt-in, it still needs documentation.

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

Pre-declaring streams seems like a good idea in general (and it solves some problems) so in #1542 we made the built in plans do so by default.  However this exposed some ophyd objects (notable AD) that were using the `trigger` method to sort out what the keys will be.  If you call `describe` before `trigger` you get different answers so going all-in on this by default is a bit too aggressive.

closes #1609 

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->

Added tests and modified two test that assumed the existence of pre-declare a bit too hard to easily un do. 

<!--
## Screenshots (if appropriate):
-->


Patch:
diff --git a/bluesky/plans.py b/bluesky/plans.py
index 1aff6203..c2011657 100644
--- a/bluesky/plans.py
+++ b/bluesky/plans.py
@@ -5,6 +5,7 @@ from functools import partial
 import collections
 from collections import defaultdict
 import time
+import os
 
 import numpy as np
 try:
@@ -66,7 +67,7 @@ def count(detectors, num=1, delay=None, *, per_shot=None, md=None):
     _md['hints'].setdefault('dimensions', [(('time',), 'primary')])
 
     # per_shot might define a different stream, so do not predeclare primary
-    predeclare = (per_shot is None)
+    predeclare = (per_shot is None and os.environ.get('BLUESKY_PREDECLARE', False))
     if per_shot is None:
         per_shot = bps.one_shot
 
@@ -514,7 +515,7 @@ def log_scan(detectors, motor, start, stop, num, *, per_step=None, md=None):
     else:
         _md['hints'].setdefault('dimensions', dimensions)
 
-    predeclare = (per_step is None)
+    predeclare = (per_step is None and os.environ.get('BLUESKY_PREDECLARE', False))
     if per_step is None:
         per_step = bps.one_1d_step
 
@@ -645,7 +646,8 @@ def adaptive_scan(detectors, target_field, motor, start, stop,
         else:
             direction_sign = -1
         devices = tuple(utils.separate_devices(detectors + [motor]))
-        yield from bps.declare_stream(*devices, name='primary')
+        if os.environ.get('BLUESKY_PREDECLARE', False):
+            yield from bps.declare_stream(*devices, name='primary')
         while next_pos * direction_sign < stop * direction_sign:
             yield Msg('checkpoint')
             yield from bps.mv(motor, next_pos)
@@ -838,8 +840,8 @@ def tune_centroid(
         cur_I = None
         sum_I = 0       # for peak centroid calculation, I(x)
         sum_xI = 0
-
-        yield from bps.declare_stream(motor, *detectors, name='primary')
+        if os.environ.get('BLUESKY_PREDECLARE', False):
+            yield from bps.declare_stream(motor, *detectors, name='primary')
         while abs(step) >= min_step and low_limit <= next_pos <= high_limit:
             yield Msg('checkpoint')
             yield from bps.mv(motor, next_pos)
@@ -931,7 +933,7 @@ def scan_nd(detectors, cycler, *, per_step=None, md=None):
         # change it, else set it to the one generated above
         _md['hints'].setdefault('dimensions', dimensions)
 
-    predeclare = (per_step is None)
+    predeclare = (per_step is None and os.environ.get('BLUESKY_PREDECLARE', False))
     if per_step is None:
         per_step = bps.one_nd_step
     else:


Test patch:
diff --git a/bluesky/tests/test_plans.py b/bluesky/tests/test_plans.py
index 207e4865..a48dc0ec 100644
--- a/bluesky/tests/test_plans.py
+++ b/bluesky/tests/test_plans.py
@@ -197,7 +197,8 @@ def test_relative_pseudo(hw, RE, db):
     assert (tb1[get_hint(p.pseudo1)] == np.linspace(0, 2, 5)).all()
 
 
-def test_reset_wrapper(hw, RE):
+def test_reset_wrapper(hw, RE, monkeypatch):
+    monkeypatch.setenv('BLUESKY_PREDECLARE', '1')
     p = hw.pseudo3x3
     m_col = MsgCollector()
     RE.msg_hook = m_col
@@ -717,7 +718,8 @@ def test_describe_failure(RE):
     st.verify()
 
 
-def test_errors_through_msg_mutator(hw):
+def test_errors_through_msg_mutator(hw, monkeypatch):
+    monkeypatch.setenv('BLUESKY_PREDECLARE', '1')
     gen = bp.rel_scan([], hw.motor, 5, -5, 10)
 
     msgs = []
@@ -749,3 +751,20 @@ def test_errors_through_msg_mutator(hw):
     ]
 
     assert target == [m.command for m in msgs]
+
+
+@pytest.mark.parametrize('predeclare', [True, False])
+def test_predeclare_env(hw, monkeypatch, predeclare):
+    from cycler import cycler
+
+    if predeclare:
+        monkeypatch.setenv('BLUESKY_PREDECLARE', '1')
+
+    for p in [bp.count([hw.det]),
+              bp.scan_nd([hw.det], cycler(hw.motor1, [1, 2, 3]) * cycler(hw.motor2, [4, 5, 6])),
+              bp.log_scan([hw.det], hw.motor, 1, 20, 5)]:
+        cmds = [m.command for m in p]
+        if predeclare:
+            assert 'declare_stream' in cmds
+        else:
+            assert 'declare_stream' not in cmds
diff --git a/bluesky/tests/test_run_engine.py b/bluesky/tests/test_run_engine.py
index b88357a2..1d9f2274 100644
--- a/bluesky/tests/test_run_engine.py
+++ b/bluesky/tests/test_run_engine.py
@@ -1588,8 +1588,10 @@ def test_double_call(RE):
     assert uid1 != uid2
 
 
-def test_num_events(RE, hw, db):
-
+@pytest.mark.parametrize('predeclare', [True, False])
+def test_num_events(RE, hw, db, predeclare, monkeypatch):
+    if predeclare:
+        monkeypatch.setenv('BLUESKY_PREDECLARE', '1')
     RE.subscribe(db.insert)
 
     rs1 = RE(count([]))
@@ -1598,7 +1600,10 @@ def test_num_events(RE, hw, db):
     else:
         uid1 = rs1[0]
     h = db[uid1]
-    assert h.stop['num_events'] == {'primary': 0}
+    if predeclare:
+        assert h.stop['num_events'] == {'primary': 0}
+    else:
+        assert h.stop['num_events'] == {}
 
     rs2 = RE(count([hw.det], 5))
     if RE.call_returns_result:
@@ -1617,7 +1622,10 @@ def test_num_events(RE, hw, db):
     else:
         uid3 = rs3[0]
     h = db[uid3]
-    assert h.stop['num_events'] == {'primary': 0, 'baseline': 2}
+    if predeclare:
+        assert h.stop['num_events'] == {'primary': 0, 'baseline': 2}
+    else:
+        assert h.stop['num_events'] == {'baseline': 2}
 
     rs4 = RE(count([hw.det], 5))
     if RE.call_returns_result: