BUG: Fix incorrect enter/exit called after scripted module reload

[Sunderlandkyl]

When a scripted loadable module was reloaded using slicer.util.reloadScriptedModule, pythonSource was not changed in qSlicerScriptedLoadableModuleWidget. As a result, the previous enter/exit methods continued to be used by the scripted module.

Fixed by making setPythonSource accessible in Python, and using it to instantiate the new widget object.

Fix #7211

[pieper]

My comment is just an idea. If you need this feature I'm okay with merging as-is.

[jcfr]

If we remove the use of the reloaded_module ivar, we should also revisit

Slicer/Base/Python/slicer/util.py

Lines 1346 to 1356 in 14a4056

	with open(filePath, encoding="utf8") as fp:
	reloaded_module = imp.load_module(
	moduleName, fp, filePath, (".py", "r", imp.PY_SOURCE))
	# find and hide the existing widget
	parent = eval("slicer.modules.%s.widgetRepresentation()" % moduleName.lower())
	for child in parent.children():
	try:
	child.hide()
	except AttributeError:
	pass

[pieper]

I agree with @jcfr - this code never seemed like a clean implementation but as I recall it was the best I could figure out at the time. If there's a cleaner and more maintainable implementation it would be great.

[Sunderlandkyl]

Currently the imp.load_module call still seems to be required. Without it, the module is not actually reloaded.

[jcfr]

Before moving forward we should revisit code setting the reloaded_module internal variable as this is likely not needed anymore.

[jcfr]

The root cause of this issue is that each C++ instance of a scripted module maintains a reference to a qSlicerPythonCppAPI instance through the d->PythonCppAPI internal variable. When virtual functions are invoked from Python (for instance, when the module manager calls qSlicerAbstractModuleWidget::enter()), the previous reference handles are still utilized.

To comprehensively resolve this, we should implement a systematic solution by introducing a "reload()" function at the C++ level. This function will ensure that the underlying PythonCppAPI object is updated correctly. To prevent any potential "catch 22" scenarios, the reload API should be accessible via the module manager.

I will propose a fix for this issue.

[lassoan]

@Sunderlandkyl this looks good to me overall. Could you rebase on latest main and resolve the conflicts? Thank you!

[Sunderlandkyl]

@jcfr

The root cause of this issue is that each C++ instance of a scripted module maintains a reference to a qSlicerPythonCppAPI instance through the d->PythonCppAPI internal variable. When virtual functions are invoked from Python (for instance, when the module manager calls qSlicerAbstractModuleWidget::enter()), the previous reference handles are still utilized.

To comprehensively resolve this, we should implement a systematic solution by introducing a "reload()" function at the C++ level. This function will ensure that the underlying PythonCppAPI object is updated correctly. To prevent any potential "catch 22" scenarios, the reload API should be accessible via the module manager.

I will propose a fix for this issue.

Should I keep this PR open. or close it in favor of a more comprehensive solution?

[jcfr]

Should I keep this PR open. or close it in favor of a more comprehensive solution?

I will follow-up this afternoon by either:

• creating a pull-request proposing an "cleaner" approach
• or slightly cleaning up this pull-request and creating an issue documenting the "proper" workaround

In the meantime, I just rebased and force-pushed the topic against the current main