-
Notifications
You must be signed in to change notification settings - Fork 2
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
Problem in adding multiple Python modules to ModuleManager #328
Comments
When python |
Long story short, rather than relying on type info for versioning modules we should do this better. Right now PluginPlay assumes that if two modules have the same type they contain the same version of the same implementation. In practice, this is not a sufficient check. What should probably happen is that we introduce a new class say If you're going to tackle this now I'd probably quickly sketch out a design doc before you do it. In particular you'll need to work out how the Python vs. C++ versions will work (off the top of my head, I'd probably go polymorphic PIMPL) and also try to foresee what other versioning info we may need. |
Yeah, I don't plan on addressing this myself right now. Just wanted to put what I'd found out there. If someone else wants to tackle it, cool. |
I have been observing intermittent errors with Python modules like
IndexError: map::at
withmod.change_input()
when I have more than one Python module in the module manager. The problem never happens if I have only one Python module, but happens randomly if I have more than one. I think the code below shows the root of the problem.If you run the Python code above several times, you will get something like:
Somehow in the second run,
test_module
s that satisfy different property types are stored at the same memory location. Module comparison always returns True might be because the comparison operator not being exported, but I think main issue is that Python modules are not added to the module manager properly. Maybe this issue is related to #309 somehow.The text was updated successfully, but these errors were encountered: