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

Fix scripted loadable module importing wrapped classes #7446

Merged

Conversation

jcfr
Copy link
Member

@jcfr jcfr commented Dec 7, 2023

  1. Fixes Endoscopy module dependency by adding "Markups"

  2. Ensure scripted modules can always import wrapped C++ classes even if loading of loadable module is disabled. It fixes the tests py_nowarning_[no]mainwindow_[nocli_]noloadableTest.

  3. De-duplicate code in loadable module factory by reusing qSlicerScriptedUtils.

@jcfr jcfr added the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Dec 7, 2023
…nction

This is a follow-up to f8fc53b (BUG: Ensure scripted module can depend on
wrapped C++ classes from other modules). It eliminates duplicated code by re-using
qSlicerScriptedUtils::importModulePythonExtensions in the loadable
module factory.

Additionally, unrelated cleanup is implemented by removing the use of Q_ASSERT
in the modulePaths() function.
This commit reuses the functionality introduced in 2ca77a8 (ENH: Support
scripted module associated only with loadable logics) by extending it to the
scripted loadable module factory (in additional of the loadable module factory).
Now, Python-wrapped C++ classes implemented by loadable modules (Logic, MRML,
MRMLDisplayableManager, VTKWidgets, PythonQt) can be imported in scripted modules,
even when the loading of loadable modules is disabled.

The commit addresses test failures related to the unavailability of the
`slicer.vtkMRMLMarkupsCurveNode` type annotation during module discovery
in tests such as:
* py_nowarning_mainwindow_noloadableTest
* py_nowarning_mainwindow_nocli_noloadableTest
* py_nowarning_nomainwindow_noloadableTest
* py_nowarning_nomainwindow_nocli_noloadableTest

Additionally, unrelated cleanup is implemented by removing the use of Q_ASSERT
in the modulePaths() function.
@jcfr jcfr force-pushed the fix-scripted-loadable-module-importing-wrapped-classes branch from 885d476 to 9386e5c Compare December 7, 2023 05:47
@jcfr
Copy link
Member Author

jcfr commented Dec 7, 2023

Given the following:

  • Successful manual builds on the Linux, macOS, and Windows factory branches ✅
  • The application starts as expected ✅
  • Previously failing tests are now passing ✅

I am forgoing our standard checks to enable a comprehensive evaluation during the upcoming round of nightly builds. This will ensure thorough testing of the changes before considering backporting to the 5.6 branch 🚀

@jcfr jcfr merged commit d9bf69c into Slicer:main Dec 7, 2023
5 checks passed
@jcfr jcfr deleted the fix-scripted-loadable-module-importing-wrapped-classes branch December 7, 2023 06:33
@jcfr jcfr removed the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant