Skip to content
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

pycromanager.Core API parity #679

Closed
tlambert03 opened this issue Sep 3, 2023 · 6 comments
Closed

pycromanager.Core API parity #679

tlambert03 opened this issue Sep 3, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@tlambert03
Copy link

tlambert03 commented Sep 3, 2023

hey @henrypinkard, I was looking into what it would take for pymmcore-widgets to control a pycromanager Core instance instead of a pymmcore instance (for example, to have a Qt DeviceProperty browser that's controlling a headless java core). At the moment, a barrier for being able to share all of the python-side widgets to also control an MMCoreJ instance in another process is the fact that pycromanager.Core doesn't have API parity with pymmcore.CMMCore. some of the types are mapped correctly (for example, with the Demo config, calling core.getCameraDevice() returns 'Camera' as expected), but most of the container types are still returning JavaObjectShadow with a separate API (e.g. core.getLoadedDevices() returns a mmcorej_StrVector that can't be iterated without using a special API).

If a goal is to be able to use python to seamlessly transition between a pure python core backend and a java bridge backend using the same API, then I think a good place to start would be to ensure that the java bridge to MMCoreJ behaves like the swig wrapper does.

See the following script as an example test (could add a ton more methods here to assert that the bridge is behaving like pymmcore would). Note I use pymmcore_plus just because it has an easy cross-platform way to install and access micromanager, but you can update the script however you like to change that. run with PYCRO=1 to test the pycromanager side, or without it to test the pymmcore side.

import os
from typing import Any, NamedTuple

import pymmcore

# need to get micromanager.  using pymmcore_plus as a convenience here,
# but you can swap it out however you like
try:
    from pymmcore_plus import find_micromanager

    mm_path = find_micromanager()
    if not mm_path:
        raise RuntimeError("Please run mmcore install.")

except ImportError:
    raise ImportError(
        "either install pymmcore_plus and run mmcore install, or "
        "modify this script to point to your micromanager installation"
    ) from None

config_file = os.path.join(mm_path, "MMConfig_demo.cfg")

# run this script with PYCRO=1 to test pycromanagers Bridge API consistency
if os.getenv("PYCRO"):
    from pycromanager import Core, start_headless

    start_headless(mm_path, config_file)
    core = Core(convert_camel_case=False)

else:
    core = pymmcore.CMMCore()


def check_func_call(core: Any, method_name: str, args: tuple, expect: Any) -> None:
    """Check that a call to core returns the expected result."""
    try:
        result = getattr(core, method_name)(*args)
    except Exception as e:
        raise AssertionError(f"Failed to call {method_name!r}:\n{e}") from None

    if isinstance(expect, ResultType):
        if not isinstance(result, expect.origin) or (
            expect.args and not all(isinstance(x, expect.args) for x in result)
        ):
            raise AssertionError(
                f"Failed {method_name!r}:\n{result!r} is not instance of {expect.args!r}"
            )
    elif type(result) != type(expect):
        raise AssertionError(
            f"{method_name!r} returned wrong type:\n{type(result)!r} != {type(expect)!r}"
        )
    elif result != expect:
        raise AssertionError(
            f"{method_name!r} returned wrong value:\n{result!r} != {expect!r}"
        )


class ResultType(NamedTuple):
    """Just assert the result type."""

    origin: type
    args: type | None = None


for method_name, args, expect in [
    ("getLoadedDevices", (), ("Core",)),
    ("getDeviceAdapterNames", (), ()),
    ("setDeviceAdapterSearchPaths", ([mm_path],), None),
    ("loadSystemConfiguration", (config_file,), None),
    ("getLoadedDevices", (), ResultType(tuple, str)),
    ("getCameraDevice", (), "Camera"),
    ("getXYStageDevice", (), "XY"),
    ("getAvailableDevices", ("DemoCamera",), ResultType(tuple, str)),
    ("getLoadedDevicesOfType", (pymmcore.CameraDevice,), ("Camera",)),
    ("getLoadedDevicesOfType", (pymmcore.HubDevice,), ("DHub",)),
    ("getDeviceDescription", ("Camera",), "Demo camera"),
    ("getDeviceType", ("Camera",), pymmcore.CameraDevice),
    ("getDevicePropertyNames", ("Autofocus",), ("Description", "HubID", "Name")),
    ("unloadDevice", ("Camera",), None),
    ("getExposure", (), 0.0),
    ("loadDevice", ("Camera", "DemoCamera", "DCam"), None),
    ("initializeDevice", ("Camera",), None),
    ("getProperty", ("Camera", "Binning"), "1"),
    ("setProperty", ("Camera", "Binning", "2"), None),
    ("getProperty", ("Camera", "Binning"), "2"),
    ("getAllowedPropertyValues", ("Camera", "Binning"), ("1", "2", "4", "8")),
    ("getState", ("Objective",), 1),
    ("getStateLabel", ("Objective",), "Nikon 10X S Fluor"),
    (
        "getStateLabels",
        ("Dichroic",),
        (
            "400DCLP",
            "Q505LP",
            "Q585LP",
            "89402bs",
            "State-4",
            "State-5",
            "State-6",
            "State-7",
            "State-8",
            "State-9",
        ),
    ),
    ("unloadAllDevices", (), None),
    # etc ...
]:
    check_func_call(core, method_name, args, expect)

there are some additional things that I would have to do in pymmcore-widgets (such as not using many of the convenience APIs we've built on top of pymmcore), but those are all doable if the underlying core objects behaved the same...

This general concept extends as well to anything we're doing in pymmcore-plus/widgets. If we standardize on the core object itself, then parity should come pretty easily

cc @nicost @marktsuchida

@tlambert03 tlambert03 added the enhancement New feature or request label Sep 3, 2023
@henrypinkard
Copy link
Member

I believe this difference comes from the differences in the java and python SWIG wrappers of the cores. I don't know the historical reasons of why these two differ. It would be nice to have a common API between the two, but not breaking backwards compatibility is a big concern here, as well as the layer in which these fixes should occur. Certainly, this would be cleaner with new, lower-level APIs for the device layer. Maybe Mark has some insight here.

@tlambert03
Copy link
Author

tlambert03 commented Sep 5, 2023

I don't know the historical reasons of why these two differ

I think that's just how SWIG works with templates. This line in MMCoreJ.i is performing the same task as the corresponding line in pymmcore ... which is to convert the C++ std::vector<std::string> template into the respective interface for each language. So i think the SWIG wrappers are doing exactly what they should be doing.

My point came more from the perspective of addressing your goal of being able to start in python, and then control either a python core or a java core, swapping back and forth seamlessly (for example, to be able to use pymmcore-widgets to control either a pure python-stack engine, or to then switch and control a Java-side engine).

To achieve that goal, i think the single most valuable object would be a Core bridge that implements python behavior on the python side, and calls java behavior on the java side. So, rather than bringing the Java Vector<String> behavior over to python, convert a python tuple[str] interface to the appropriate getters/setters on the java bridge. With that, you could swap a pymmcore object for a pycromanager.Core object anywhere, and that would just be awesome.

not breaking backwards compatibility is a big concern here

that would be easy enough with an additional flag to your __new__ constructor. That is, you already have a flexible API that depends on the end-users decisions (such as convert_camel_case), so adding a implement_python_interface flag that converts vectors to a python object that property implements __iter__ (like a plain tuple or anything) could be opt-in as well... (but also, wouldn't you agree it's the more "natural" behavior for someone working on the python side? perhaps you could aim to deprecate the usage of the java APIs on the python side, and eventually make native python behavior be the default)

Certainly, this would be cleaner with new, lower-level APIs for the device layer

I feel like it's teed up and ready to go already no? Just need to add a bit more logic to your bridge object to properly convert a few additional types?

@tlambert03
Copy link
Author

i looked into adding this feature, and while digging through the Bridge constructor code, I found that @ilyasdc already implemented this in #372 ... but I don't see any user-accessible way to set _Bridge arguments via the Core constructor.

@henrypinkard
Copy link
Member

To achieve that goal, i think the single most valuable object would be a Core bridge that implements python behavior on the python side, and calls java behavior on the java side. So, rather than bringing the Java Vector<String> behavior over to python,

Yes, I think that is a good idea

(but also, wouldn't you agree it's the more "natural" behavior for someone working on the python side? perhaps you could aim to deprecate the usage of the java APIs on the python side, and eventually make native python behavior be the default)

Yes, true. the camel case conversion is already a step in this direction. I've just partially done some of this (but partially in the reverse direction of matching pymmcore to mmcorej) in https://github.com/henrypinkard/pycro-manager/blob/43ccdfe50b6966f4004bee8266b7d9386880c83b/pycromanager/headless.py#L27

I feel like it's teed up and ready to go already no? Just need to add a bit more logic to your bridge object to properly convert a few additional types?

One complication. I would like to eventually split the Java-python bridge into its own repo, because i think it would be useful for other projects and is very much a separate module from what is on top of it. And the bridge and the other stuff are nearly if not entirely independent in the code base. So this should probably be done in such a way to avoid hard coding things about mmcoreJ in the bridge

I don't see any user-accessible way to set _Bridge arguments via the Core constructor.

I think it just needs to be exposed as an argument via JavaObject

@henrypinkard
Copy link
Member

I'm focused on other projects in the near term and won't have the bandwidth to look into this, but I think this is a good idea, and if you want to take it on, pull requests welcome!

@henrypinkard
Copy link
Member

FYI the Core functionality has been split out of pycro-manager to a new separate repo (https://github.com/micro-manager/mmpycorex) I don't have any plans to work on this in the forseeable future, but if you are interested, pull requests welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants