Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_base
-
Labels:
-
Story Points:3
-
Epic Link:
-
Sprint:Science Pipelines DM-W15-1
-
Team:Data Release Production
Description
meas_base currently has a single Swig library (like most packages), defined within a single .i file (like some packages). It also registers all of its plugins in a single python module, plugins.py.
Instead, it should:
- Have two Swig libraries: one for the interfaces and helper classes, and one for plugin algorithms. Most downstream packages will only want to %import (and hence #include) the interface, and having them build against everything slows the build down unnecessarily. The package _init_.py should import all symbols from both libraries, so the change would be transparent to the user.
- Have separate .i files for each algorithm or small group of algorithms. Each of these could %import the interface library file and the pure-Python registry code, and then register the plugins wrapped there within a %pythoncode block. That'd make the implementation of the algorithms a bit less scattered throughout the package, making them easier to maintain and better examples for new plugins.
Attachments
Activity
At this point, I don't think we should worry about it. I think that Swig macro will go away with virtually any sensible C++ redesign, and I think we'll be doing that at the beginning of the next sprint, if not sooner.
The scope of this ticket was cut down after we failed to find a way to make the wrapper macros in baseLib.i work against a second SWIG dynamic library in the same directory. The technique of importing baseLib.i works from outside lsst/meas/base, but not for a second dynamic library in the same module.
So the only change is to move all the algorithm related stuff into pluginsLib.i, and include that in baseLib.i
Code is in meas_base, u/pgee/DM-1217. baseLib.i was changed to include pluginsLib.i
Perry Gee
added a comment - The scope of this ticket was cut down after we failed to find a way to make the wrapper macros in baseLib.i work against a second SWIG dynamic library in the same directory. The technique of importing baseLib.i works from outside lsst/meas/base, but not for a second dynamic library in the same module.
So the only change is to move all the algorithm related stuff into pluginsLib.i, and include that in baseLib.i
Code is in meas_base, u/pgee/ DM-1217 . baseLib.i was changed to include pluginsLib.i One quibble: if we're not building pluginsLib.i on its own, we should rename it to just "plugins.i", as I think the "Lib" suffix tends to imply that it's a separate Swig build.
True, though it did once have its own dll. I will change the name.
A couple of comments. As I told you, I did test that I could build a completely separate library an import baseLib.i with it. It works when the library is outside of lsst.meas.base, but not if it is inside. That's the SWIG issue you helped me detect. However, just for the record, this external Lib requires a slightly different pythoncode, with an import.meas.base in it.
The other thing is not I did not move the plugin registration out of plugins.py, as it cannot be done in pythoncode when the plugin is in lsst.meas.base (due to the fact that the python wrapper class is not available at that point). And for the meas_base plugins, plugins.py seems like as good a place as any to create the wrapper. We can revisit this after the C++ redesign if this isn't what you want.
It does work when the plugin is in an external package to create the wrapper in pythoncode.
Perry Gee
added a comment - True, though it did once have its own dll. I will change the name.
A couple of comments. As I told you, I did test that I could build a completely separate library an import baseLib.i with it. It works when the library is outside of lsst.meas.base, but not if it is inside. That's the SWIG issue you helped me detect. However, just for the record, this external Lib requires a slightly different pythoncode, with an import.meas.base in it.
The other thing is not I did not move the plugin registration out of plugins.py, as it cannot be done in pythoncode when the plugin is in lsst.meas.base (due to the fact that the python wrapper class is not available at that point). And for the meas_base plugins, plugins.py seems like as good a place as any to create the wrapper. We can revisit this after the C++ redesign if this isn't what you want.
It does work when the plugin is in an external package to create the wrapper in pythoncode.
This looks like a nice, simple cleanup.
Jim:
I was not able to find a solution to the SWIG problems I encountered when I tried to put the plugins into a separate swig library from the Results, ResultMappers, and Inputs. So as we talked about on Sunday, I put the plugins in separate .i files but included them in baseLib.i
I also tried the experiment of creating a plugin in a separate module and importing baseLib.i. I found that this will only work if I include a line in the pythoncode in wrapMeasurementAlgorithmEx to do:
from lsst.meas.base import *
The reason is that if CONTROL or INPUT are defined in lsst.meas.base, they are not imported by the outside module swig library and are not found when the pythoncode is executed.
I would like a better solution to this problem, if you have one.