Repository: bluesky/bluesky (python)

Issue:
check of isinstance(obj, Movable) only accepts class attributes, rejecting potentially valid objects
I'm trying to get yaqc-bluesky settable objects to work in a `grid_scan`, but bluesky is claiming that my arguments are invalid. In particular, my objects are failing to pass `isinstance(obj, Movable)` [1].

The `Movable` class is an abstract base class [2] which, according to the comment, should turn the `isinstance` check into, essentially, a check against the list of expected attributes. Not actually understanding the ABC magic, all I can say is that I can pass `hasattr` without passing `isinstance`:

```python
import bluesky
from bluesky.utils import Movable


EXPECTED_ATTRS = (
    'name',
    'parent',
    'read',
    'describe',
    'read_configuration',
    'describe_configuration',
    'set',
    'stop',
)


class Device(object):

    def __init__(self):
        for attr in EXPECTED_ATTRS:
            setattr(self, attr, None)


device = Device()
print("hasattr", all(hasattr(device, attr) for attr in EXPECTED_ATTRS))  # True
print("isinstance", isinstance(device, Movable))  # False
print("subclasshook", Movable.__subclasshook__(device))  # True
```

Even more strangely, when I play with your example notebook [3] your own ophyd motor objects also fail this `isinstance` check and, indeed, are rejected within `grid_scan` (Hello Bluesky.ipynb). Is it possible that this plan is just broken?

Any guidance would be appreciated---thanks! :smile: 

[1] https://github.com/bluesky/bluesky/blob/master/bluesky/plan_patterns.py#L431
[2] https://github.com/bluesky/bluesky/blob/master/bluesky/utils.py#L1519-L1548
[3] https://try.nsls2.bnl.gov/

Pull request:
ENH: allow "movable" objects that match bluesky interface with instance attributes
This PR changes the way that bluesky checks if an argument is "movable". Before: `isinstance(obj, Movable)`, now: `is_movable(obj)`.

Current behavior requires all of the expected attributes to be implemented as class attributes. With this PR, bluesky now checks instance attributes. For example, a class that defines `name` as an instance attribute at init time would have failed in old state but now passes. All objects that passed before should still pass.

closes #1377 

As an example of where current state gets weird, try running the following cell in a new notebook on https://try.nsls2.bnl.gov/

```python
from bluesky.utils import Movable
from bluesky.plans import grid_scan
from bluesky import RunEngine

%run scripts/beamline_configuration.py
motor = motor_ph
det = ph

RE(grid_scan([det], motor, 0, 1, 11))  # fails
```

Patch:
diff --git a/bluesky/plan_patterns.py b/bluesky/plan_patterns.py
index 303c79a5..a4f6f46e 100644
--- a/bluesky/plan_patterns.py
+++ b/bluesky/plan_patterns.py
@@ -11,7 +11,7 @@ try:
 except ImportError:
     from toolz import partition
 
-from .utils import snake_cyclers, Movable
+from .utils import snake_cyclers, is_movable
 
 
 def spiral(x_motor, y_motor, x_start, y_start, x_range, y_range, dr, nth, *,
@@ -428,7 +428,7 @@ def classify_outer_product_args_pattern(args):
             raise ValueError(f"Unknown pattern '{pattern}'")
         for n, element in enumerate(args):
             # Check if the element is the motor
-            flag = isinstance(element, Movable)
+            flag = is_movable(element)
             # If the element is expected to be the motor, then flip the flag
             if n in pos_movable:
                 flag = not flag
diff --git a/bluesky/utils.py b/bluesky/utils.py
index fabbd53a..956ad788 100644
--- a/bluesky/utils.py
+++ b/bluesky/utils.py
@@ -1536,6 +1536,32 @@ def _rearrange_into_parallel_dicts(readings):
     return data, timestamps
 
 
+def is_movable(obj):
+    """Check if object satisfies bluesky 'movable' interface.
+
+    Parameters
+    ----------
+    obj : Object
+        Object to test.
+
+    Returns
+    -------
+    boolean
+        True if movable, False otherwise.
+    """
+    EXPECTED_ATTRS = (
+        'name',
+        'parent',
+        'read',
+        'describe',
+        'read_configuration',
+        'describe_configuration',
+        'set',
+        'stop',
+    )
+    return all(hasattr(obj, attr) for attr in EXPECTED_ATTRS)
+
+
 class Movable(metaclass=abc.ABCMeta):
     """
     Abstract base class for objects that satisfy the bluesky 'movable' interface.
@@ -1549,12 +1575,15 @@ class Movable(metaclass=abc.ABCMeta):
         # We need to detect if 'm' is a motor
         if isinstance(m, Movable):
             print(f"The object {m.name} is a motor")
-
     """
     @classmethod
     def __subclasshook__(cls, C):
         # If the following condition is True, the object C is recognized
         # to have Movable interface (e.g. a motor)
+        msg = """The Movable abstract base class is deprecated and will be removed in a future
+                 version of bluesky. Please use bluesky.utils.is_movable(obj) to test if an object
+                 satisfies the movable interface."""
+        warnings.warn(msg, DeprecationWarning)
         EXPECTED_ATTRS = (
             'name',
             'parent',
diff --git a/doc/source/hardware.rst b/doc/source/hardware.rst
index 01b4a9db..94f9d0b0 100644
--- a/doc/source/hardware.rst
+++ b/doc/source/hardware.rst
@@ -219,7 +219,8 @@ Settable (Movable) Device
 +++++++++++++++++++++++++
 
 The interface of a settable device extends the interface of a readable device
-with the following additional methods and attributes:
+with the following additional methods and attributes.
+Settable device objects must pass ``bluesky.utils.is_movable(obj)``.
 
 .. class:: SettableDevice:
 


Test patch:
diff --git a/bluesky/tests/test_utils.py b/bluesky/tests/test_utils.py
index 69cba2a4..c894e294 100644
--- a/bluesky/tests/test_utils.py
+++ b/bluesky/tests/test_utils.py
@@ -4,7 +4,7 @@ import numpy as np
 from functools import reduce
 import operator
 
-from bluesky.utils import ensure_generator, Msg, merge_cycler, Movable, CallbackRegistry
+from bluesky.utils import ensure_generator, Msg, merge_cycler, is_movable, CallbackRegistry
 from cycler import cycler
 
 
@@ -103,12 +103,12 @@ def test_cycler_merge_mixed(hw, children):
     assert mcyc.by_key() == cyc.by_key()
 
 
-def test_Movable(hw):
+def test_is_movable(hw):
 
     obj_list = [(10, False), (1.05, False), ("some_string", False),
                 (hw.det, False), (hw.motor, True)]
     for obj, result in obj_list:
-        assert isinstance(obj, Movable) == result, \
+        assert is_movable(obj) == result, \
             f"The object {obj} is incorrectly recognized "\
             f"as {'' if result else 'not '}movable"