Improve robustness of Python completion model #1229
Conversation
Remove the `dict` variable from `ctkAbstractPythonManager::pythonAttributes` as it was redundant. All relevant operations now directly use object.
7bc9d81 to
c741fd9
Compare
|
It all looks good except hiding |
| else | ||
| { | ||
| // failed to invoke callable, no completions are available | ||
| PyErr_Clear(); |
There was a problem hiding this comment.
| PyErr_Clear(); | |
| this->handleErrorAsWarning(); |
Instead of blindly discarding error ... we could introduce a new function handleErrorAsWarning allowing those to be logged while avoiding them to be reported in the Python console.
There was a problem hiding this comment.
Attempt to replace the calls PyErr_Clear(); with PythonQt::self()->handleError(); allows to identify the "problematic" properties but polluted the output:
There was a problem hiding this comment.
Using the updated VTK version in Slicer/Slicer#8238, I can confirm that SetInterpolate is showing again:
I'm not sure what excludeGetSet is really doing here as I see both the property and Getter related to getting the Slicer node ID. Unless this is an artifact of doing an incremental build rather than a fresh clean build.
|
After adding dn = n.GetDisplayNode()
appendParenthesis = False
excludeGetSet = False
withGetSets = set(slicer.app.pythonManager().pythonAttributes("dn", "__main__", appendParenthesis, excludeGetSet))
excludeGetSet = True
withoutGetSets = set(slicer.app.pythonManager().pythonAttributes("dn", "__main__", appendParenthesis, excludeGetSet))
print(withGetSets - withoutGetSets)There are few issues:
As pointed out by @lassoan excluding the property from completion will be confusing, instead we want them to be listed by with a lower priority (e.g at the end) |
|
Yeah putting the older Get/Set methods at the end could be a good way to discourage that auto-complete in favor of the more python property ones as part of a transition period starting now. Then ultimately the Get/Set autocompletion could be removed in the future. ITK had already switched to the more python interface as part of ITK 5 (https://discourse.itk.org/t/itk-5-0-beta-1-pythonic-interface/1271). |
…ython This may be useful to inspect the input provided to the Python completion model.
…_object` Clear error if attempt to get an object or dictionary attribute failed while iterating over items.
Ensure `classToInstantiate` is properly managed by incrementing its reference count when retrieved via `PyDict_GetItemString`, which returns a borrowed reference. This allows safely calling `Py_DECREF` afterward, regardless of whether the object came from a dictionary or via `PyObject_GetAttrString`, whic 8000 h returns a new reference.
c741fd9 to
c1ed918
Compare
|
Frmo @lassoan
From @jamesobutler
It turns out that those methods are already added last. I just removed the attempt to remove the This topic is now ready for integration 🚀 |
|
@lassoan @jamesobutler Once this is approved & integrated, I will then:
|
This pull request includes a series of improvements and bug fixes to the
ctkAbstractPythonManager::pythonAttributesand related methods improving the autocompletion model. The changes focus on simplifying the code, improving memory management, and ensuring more accurate and error-resilient behavior when inspecting Python objects.Exclude property-like accessors from autocompletionclassToInstantiatedictvariableRelated pull request: