henrypinkard / Pygellan

[DEPREACATED] Python interface for data-driven microscopy with Micro-manager/Micro-Magellan
BSD 3-Clause "New" or "Revised" License
14 stars 3 forks source link

separation of low-level micromanager-ZMQ interface from pygellan #31

Open bryantChhun opened 4 years ago

bryantChhun commented 4 years ago

@henrypinkard @nicost Furthering the disucssion from #25

I still hold that the "core" behavior that we have should be separated from the magellan acquisition engine for precisely these reasons:

In order to enable the functionality described in the issues, I will need to add different types of socket architectures (i.e. not Java server python client). These may be fairly tightly coupled to the magellan acquisition engine. Hard to say for me right now which layer they would belong in, because there is still a lot to figure out.

The problem with this approach is that it means future development potentially breaks existing behavior. If you can guarantee it will not, then that's actually an argument that this "core" functionality is ready to be isolated.

In other words, i'm saying that the goal of this module is to provide a general service that others can build upon for their applications. Specific elaborations should build on top as a separate layer, but mostly pipe into API provided by this "core". If you think certain fundamental changes need to be made (multi sockets maybe?), then let's think about that right now.

nicost commented 4 years ago

I agree with Bryant's thinking. It looks to me that the current functionality is good enough to merge into master, but if you are most likely to change the interface in the near future, then we should hold off.

henrypinkard commented 4 years ago

Yes, I agree also. However, I don't think there's much danger for breaking the existing API with future changes, namely because there isn't much of an API that exists at this point anyway. It's just creating the bridge, and getting the core or whatever java plugin object. Everything else is read from Java at runtime.

Regarding future additional functionality, I think this will be mostly or entirely under the hood, so it is safe to develop on top of. I don't think this is the right time for planning out a complete roadmap of internal workings, because I don't feel I have all of the necessary understanding yet to do so. I simply don't know what I don't know, and as such I think it will be more efficient to experiment than to make a master plan that will soon require revision.

There are already people using this and developing on top of it (the previous version that's in Magellan, but functions the same from a user perspective), so I see no reason to delay the merge into master since it contains bug fixes and improvements to existing functionality. It will also resynchronize the pypi version with the current examples, which I think will help avoid confusion for users that aren't running from source.

nicost commented 4 years ago

Great! Sounds like this is ready for integration. I talked with @marktsuchida about this, and he sees no problems either. I'll work on the merge (little bit more complicated than usual as we split off the zmq branch just before the merge back into master). I like the name "pycro-manager"!