You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
On Windows, xlwings has required comtypes since at least version v0.9.0 (588f0f5) until at least the current master (dceeedd). After looking at it carefully, I think this may be unnecessary. The cost of requiring an unnecessary library is added time in installation under normal set ups and added complexity and size in packaging and distribution (see, e.g., #625, which is quite similar to my use case). Very shortly I will send a PR to accomplish my recommended changes.
The only imports from comtypes are IUnknown and IDispatch:
(accessible_object_from_window() is called only from get_xl_app_from_hwnd() as shown above and from is_hwnd_xl_app(), where nothing is done with the returned pointer.)
So it looks like there is one use of ctypes.POINTER(IDispatch) and there are two uses of ctypes.byref(IDispatch._iid_).
ctypes.POINTER(IDispatch)
Correct me if I'm wrong, but I believe this declares a pointer to a structure, and, from the code defining IDispatch, it looks like the structure has no _fields_. IDispatch inherits from our friend IUnknown, which also defines no _fields_. (The metaclass for IUnknown is defined here; notice that IUnknown._case_insensitive_ is set to False.)
Moreover, the pointer produced by POINTER(IDispatch) is only ever passed directly to oleacc.AccessibleObjectFromWindow (shown above) and to pythoncom.PyCom_PyObjectFromIUnknown. Being C++ functions and being called through ctypes suggests to me that AccessibleObjectFromWindow and PyCom_PyObjectFromIUnknown really only care about the memory address contained in the pointer and, for the byref use, the memory address at which it is stored, not any of its Python metadata it might gain by coming from POINTER(IDispatch) rather than ctypes.c_void_p. I.e., it's not like there's any type checking happening.
So I would argue that POINTER(IDispatch)() can be replaced with ctypes.c_void_p() and a comment linking to IDispatch's documentation.
ctypes.byref(IDispatch._iid_)
IDispatch._iid_ is defined here. Can this be replaced with the following? It's just the essentials from here.
On Windows, xlwings has required comtypes since at least version v0.9.0 (588f0f5) until at least the current master (dceeedd). After looking at it carefully, I think this may be unnecessary. The cost of requiring an unnecessary library is added time in installation under normal set ups and added complexity and size in packaging and distribution (see, e.g., #625, which is quite similar to my use case). Very shortly I will send a PR to accomplish my recommended changes.
The only imports from comtypes are
IUnknown
andIDispatch
:xlwings/xlwings/_xlwindows.py
Lines 29 to 30 in dceeedd
IUnknown
IUnknown
is only used incomtypes_to_pywin()
in case theinterface
argument is not set:xlwings/xlwings/_xlwindows.py
Lines 196 to 202 in dceeedd
But
comtypes_to_pywin
is only called once, inget_xl_app_from_hwnd()
, with theinterface
argument set toIDispatch
.xlwings/xlwings/_xlwindows.py
Lines 217 to 225 in dceeedd
Thus
xlwings/xlwings/_xlwindows.py
Lines 200 to 202 in dceeedd
could be replaced as
with
comtypes_to_pywin
no longer taking aninterface
argument.So the import of
IUnknown
can be removed completely without changing a thing.IDispatch
IDispatch
is used only inget_xl_app_from_hwnd()
as shown above and fromaccessible_object_from_window()
:xlwings/xlwings/_xlwindows.py
Lines 188 to 193 in dceeedd
(
accessible_object_from_window()
is called only fromget_xl_app_from_hwnd()
as shown above and fromis_hwnd_xl_app()
, where nothing is done with the returned pointer.)So it looks like there is one use of
ctypes.POINTER(IDispatch)
and there are two uses ofctypes.byref(IDispatch._iid_)
.ctypes.POINTER(IDispatch)
Correct me if I'm wrong, but I believe this declares a pointer to a structure, and, from the code defining
IDispatch
, it looks like the structure has no_fields_
.IDispatch
inherits from our friendIUnknown
, which also defines no_fields_
. (The metaclass forIUnknown
is defined here; notice thatIUnknown._case_insensitive_
is set toFalse
.)Moreover, the pointer produced by
POINTER(IDispatch)
is only ever passed directly tooleacc.AccessibleObjectFromWindow
(shown above) and topythoncom.PyCom_PyObjectFromIUnknown
. Being C++ functions and being called through ctypes suggests to me thatAccessibleObjectFromWindow
andPyCom_PyObjectFromIUnknown
really only care about the memory address contained in the pointer and, for thebyref
use, the memory address at which it is stored, not any of its Python metadata it might gain by coming fromPOINTER(IDispatch)
rather thanctypes.c_void_p
. I.e., it's not like there's any type checking happening.So I would argue that
POINTER(IDispatch)()
can be replaced withctypes.c_void_p()
and a comment linking toIDispatch
's documentation.ctypes.byref(IDispatch._iid_)
IDispatch._iid_
is defined here. Can this be replaced with the following? It's just the essentials from here.Then the two instances of
byref(IDispatch._iid_)
could be replaced withbyref(_IDISPATCH_GUID)
.The text was updated successfully, but these errors were encountered: