Iterators in generated files implementing next() instead of __next__()

[jonschz]

The following code was generated on comtypes 1.1.14 using comtypes.client.GetModule("portabledeviceapi.dll"):

class IEnumPortableDeviceObjectIDs(IUnknown):
    """IEnumPortableDeviceObjectIDs Interface"""
    [...]
    def __iter__(self):
        return self

    def next(self):
        item, fetched = self.Next(1)
        [...]

    def __getitem__(self, index):
    [...]

While trying to do some basic type annotation I realised that a __next__() method is missing, hence the return value of __iter__() is not an iterator. The best fix which won't break existing code is likely to just automatically add

    def __next__(self):
        return self.next()

to every class generated in such a way.

[junkmd]

Perhaps the related code is around there.

comtypes/comtypes/tools/codegenerator.py

Lines 809 to 859 in fed3de6

	def ComInterfaceHead(self, head):
	if head.itf.name in self.known_symbols:
	return
	base = head.itf.base
	if head.itf.base is None:
	# we don't beed to generate IUnknown
	return
	self.generate(base.get_head())
	self.more.add(base)
	basename = self.type_name(head.itf.base)
	self.need_GUID()
	if not self.last_item_class:
	print(file=self.stream)
	print(file=self.stream)
	self.last_item_class = True
	print("class %s(%s):" % (head.itf.name, basename), file=self.stream)
	doc = getattr(head.itf, "doc", None)
	if doc:
	print(self._to_docstring(doc), file=self.stream)
	print(" _case_insensitive_ = True", file=self.stream)
	print(" _iid_ = GUID(%r)" % head.itf.iid, file=self.stream)
	print(" _idlflags_ = %s" % head.itf.idlflags, file=self.stream)
	if self._is_enuminterface(head.itf):
	print(file=self.stream)
	print(" def __iter__(self):", file=self.stream)
	print(" return self", file=self.stream)
	print(file=self.stream)
	print(" def next(self):", file=self.stream)
	print(" item, fetched = self.Next(1)", file=self.stream)
	print(" if fetched:", file=self.stream)
	print(" return item", file=self.stream)
	print(" raise StopIteration", file=self.stream)
	print(file=self.stream)
	print(" def __getitem__(self, index):", file=self.stream)
	print(" self.Reset()", file=self.stream)
	print(" self.Skip(index)", file=self.stream)
	print(" item, fetched = self.Next(1)", file=self.stream)
	print(" if fetched:", file=self.stream)
	print(" return item", file=self.stream)
	print(" raise IndexError(index)", file=self.stream)
	print(file=self.stream)
	print(file=self.stream)

• comtypes/comtypes/tools/codegenerator.py

  Lines 799 to 807 in fed3de6

  	def _is_enuminterface(self, itf):
  	# Check if this is an IEnumXXX interface
  	if not itf.name.startswith("IEnum"):
  	return False
  	member_names = [mth.name for mth in itf.members]
  	for name in ("Next", "Skip", "Reset", "Clone"):
  	if name not in member_names:
  	return False
  	return True

A com object which has _NewEnum will be Iterator by metaclass and patching.

comtypes/comtypes/__init__.py

Lines 410 to 433 in fed3de6

	if has_name("_NewEnum"):
	@patcher.Patch(self)
	class _(object):
	def __iter__(self):
	"Return an iterator over the _NewEnum collection."
	# This method returns a pointer to _some_ _NewEnum interface.
	# It relies on the fact that the code generator creates next()
	# methods for them automatically.
	#
	# Better would maybe to return an object that
	# implements the Python iterator protocol, and
	# forwards the calls to the COM interface.
	enum = self._NewEnum
	if isinstance(enum, types.MethodType):
	# _NewEnum should be a propget property, with dispid -4.
	#
	# Sometimes, however, it is a method.
	enum = enum()
	if hasattr(enum, "Next"):
	return enum
	# _NewEnum returns an IUnknown pointer, QueryInterface() it to
	# IEnumVARIANT
	from comtypes.automation import IEnumVARIANT
	return enum.QueryInterface(IEnumVARIANT)

But a com object with no _NewEnum , even if they has name starts with IEnum..., would not be Iterator.

• For the __dunder__ method, it would be better to define by a unified way of doing either metaclass or generating-code.
  However, this would require man-hours to verify the impact and be large amount of the change.
  So "define by a unified way" would be outside the scope of this problem solution.

I think this issue can be resolved by only changing tools.codegenerator so that the __next__ method is defined in the class.

[junkmd]

@psisquared2

Please let me know your code that you expected to work correctly (and terminated abnormally).

I may be able to get some hints from that code as to what test cases would guarantee the behavior.

[junkmd]

@psisquared2

I am planning to add a type-stub generating process to the codegenerator module.
However, I do not plan to change the class definition in the runtime code, including the __dunder__ methods.

So you are free from the worry of code conflictions with your __next__ method definition and my type-stub generating process when you PR to resolve this issue.

[junkmd]

@psisquared2

Can you remind this?

The drop_py2 plan(#392) is ongoing.
So implementing the bridge for next in Py2 and __next__ in Py3 is no more required.

PEP 3114: the standard next() method has been renamed to __next__().
https://docs.python.org/3/whatsnew/3.0.html#operators-and-special-methods

And can you react to below?

Please let me know your code that you expected to work correctly (and terminated abnormally).

I may be able to get some hints from that code as to what test cases would guarantee the behavior.
#353 (comment)

[jonschz]

My apologies, I had a few busy months and found no time to focus on my project involving comtypes.

I am planning to add a type-stub generating process to the codegenerator module.
[...]
So implementing the bridge for next in Py2 and next in Py3 is no more required.

Both sounds very good. I suspect that one can just rename next() to __next__() as part of this plan?

Please let me know your code that you expected to work correctly (and terminated abnormally).

I put together this rough sketch based on https://github.com/KasparNagu/PortableDevices:

import ctypes
import comtypes
import comtypes.client

comtypes.client.GetModule("portabledeviceapi.dll")
comtypes.client.GetModule("portabledevicetypes.dll")
import comtypes.gen._1F001332_1A57_4934_BE31_AFFC99F4EE0A_0_1_0 as port
import comtypes.gen._2B00BA2F_E750_4BEB_9235_97142EDE1D3E_0_1_0 as types

clientInformation = comtypes.client.CreateObject(
    types.PortableDeviceValues,
    clsctx=comtypes.CLSCTX_INPROC_SERVER,
    interface=port.IPortableDeviceValues)
_device = comtypes.client.CreateObject(
    port.PortableDevice,
    clsctx=comtypes.CLSCTX_INPROC_SERVER,
    interface=port.IPortableDevice)
_device.Open('<valid device id like "\\\\?\\usb#vid_18d1&pid_4...{<GUID>}">', clientInformation)

objects = _device.Content().EnumObjects(ctypes.c_ulong(0), "DEVICE", None)

for obj in objects:
    print(obj)

This only works if I change next() to __next__() in IEnumPortableDeviceObjectIDs, otherwise it crashes with iter() returned non-iterator of type 'POINTER(IEnumPortableDeviceObjectIDs)'.

[junkmd]

Thank you for your response.

I have created the TestCase from the sample code you provided.

comtypes/comtypes/test/test_portabledevice.py

Lines 1 to 60 in 6d136d1

	import ctypes
	import unittest as ut
	import comtypes
	import comtypes.client
	comtypes.client.GetModule("portabledeviceapi.dll")
	import comtypes.gen.PortableDeviceApiLib as port_api
	comtypes.client.GetModule("portabledevicetypes.dll")
	import comtypes.gen.PortableDeviceTypesLib as port_types
	################################################################
	#
	# TODO:
	#
	# It seems bad that only external test like this can verify
	# the behavior of `comtypes` implementation.
	# Find a safe way to do this, like having a virtual portable
	# device and using the COM API for it.
	#
	################################################################
	class Test_IPortableDevice(ut.TestCase):
	# To avoid damaging or changing the environment, do not CREATE, DELETE or UPDATE!
	# Do READ only!
	def setUp(self):
	info = comtypes.client.CreateObject(
	port_types.PortableDeviceValues().IPersist_GetClassID(),
	clsctx=comtypes.CLSCTX_INPROC_SERVER,
	interface=port_types.IPortableDeviceValues,
	)
	mng = comtypes.client.CreateObject(
	port_api.PortableDeviceManager().IPersist_GetClassID(),
	clsctx=comtypes.CLSCTX_INPROC_SERVER,
	interface=port_api.IPortableDeviceManager,
	)
	p_id_cnt = ctypes.pointer(ctypes.c_ulong())
	mng.GetDevices(ctypes.POINTER(ctypes.c_wchar_p)(), p_id_cnt)
	if p_id_cnt.contents.value == 0:
	self.skipTest("There is no portable device in the environment.")
	dev_ids = (ctypes.c_wchar_p * p_id_cnt.contents.value)()
	mng.GetDevices(ctypes.cast(dev_ids, ctypes.POINTER(ctypes.c_wchar_p)), p_id_cnt)
	self.device = comtypes.client.CreateObject(
	port_api.PortableDevice().IPersist_GetClassID(),
	clsctx=comtypes.CLSCTX_INPROC_SERVER,
	interface=port_api.IPortableDevice,
	)
	self.device.Open(list(dev_ids)[0], info)
	def test_EnumObjects(self):
	enumobj = self.device.Content().EnumObjects(ctypes.c_ulong(0), "DEVICE", None)
	self.assertIsInstance(enumobj, port_api.IEnumPortableDeviceObjectIDs)
	[_ for _ in enumobj] # is iterable?
	if __name__ == "__main__":
	ut.main()

Check the test content. It does not CREATE, DELETE or UPDATE and may not damage your portable device.
This test will be skipped if there is no portable device in the environment because no assertion can be done.

Please create a new branch in your local repo from the drop_py2 and add this test module, and do followings.

1. Try python -m unittest comtypes.test.test_portabledevice -v with connecting some kind of portable device in your environment.
   If nothing is changed in the production code, this TestCase will fail.

2. Replace line 1032 in the code snippet from codegenerator with the following;

comtypes/comtypes/tools/codegenerator.py

Lines 998 to 1048 in 6d136d1

	def ComInterfaceHead(self, head: typedesc.ComInterfaceHead) -> None:
	if head.itf.name in self.known_symbols:
	return
	base = head.itf.base
	if head.itf.base is None:
	# we don't beed to generate IUnknown
	return
	self.generate(base.get_head())
	self.more.add(base)
	basename = self._to_type_name(head.itf.base)
	self.imports.add("comtypes", "GUID")
	if not self.last_item_class:
	print(file=self.stream)
	print(file=self.stream)
	self.last_item_class = True
	print("class %s(%s):" % (head.itf.name, basename), file=self.stream)
	doc = getattr(head.itf, "doc", None)
	if doc:
	print(self._to_docstring(doc), file=self.stream)
	print(" _case_insensitive_ = True", file=self.stream)
	print(" _iid_ = GUID(%r)" % head.itf.iid, file=self.stream)
	print(" _idlflags_ = %s" % head.itf.idlflags, file=self.stream)
	if self._is_enuminterface(head.itf):
	print(file=self.stream)
	print(" def __iter__(self):", file=self.stream)
	print(" return self", file=self.stream)
	print(file=self.stream)
	print(" def next(self):", file=self.stream)
	print(" item, fetched = self.Next(1)", file=self.stream)
	print(" if fetched:", file=self.stream)
	print(" return item", file=self.stream)
	print(" raise StopIteration", file=self.stream)
	print(file=self.stream)
	print(" def __getitem__(self, index):", file=self.stream)
	print(" self.Reset()", file=self.stream)
	print(" self.Skip(index)", file=self.stream)
	print(" item, fetched = self.Next(1)", file=self.stream)
	print(" if fetched:", file=self.stream)
	print(" return item", file=self.stream)
	print(" raise IndexError(index)", file=self.stream)
	print(file=self.stream)
	print(file=self.stream)

comtypes/comtypes/tools/codegenerator.py

Line 1032 in 6d136d1

print(" def next(self):", file=self.stream)

↓

            print("    def __next__(self):", file=self.stream)

3. Run python -m clear_comtypes_cache -y to clear caches and the .../comtypes/gen directory.

4. Re-run python -m unittest comtypes.test.test_portabledevice -v.
   If the test passes without being skipped, it is the success!

Also, if you know, please let me know safe way to do this, like having a virtual portable device and using the COM API for it.

[jonschz]

I did the test as you described and I can confirm the correct behaviour:

• Without the code change: the test fails with the aforementioned error message.
• With the code change: the test passes.
• Without a portable device connected: the test is skipped.

Unfortunately I don't have the backgrounds in the WPD API to give advice on how to unit-test the code without a device connected.

As far as I am concerned this issue can be closed, but maybe you want to keep it open until drop_py2 is merged into master. Thanks for fixing it!

[junkmd]

@jonschz

As you may have guessed, I will keep this issue open at least until a commit is added that makes changes to the production codebase.

Are you interested in contributing to this project?
I am a collaborator on this project, so I can review and merge the code you changed in the codegenerator.

[jonschz]

Thanks, I appreciate the opportunity - too often have I seen collaborators just fixing issues without acknowledgement even in cases where the contribution was significantly larger than mine here.

[junkmd]

I am interested in a healthy expansion of the OSS community.

Such as this case, the ideal is for those who find a problem to solve it with the help of the community and submit a PR for the fix.

I believe that there are more eyes👀, mouths👄, ears👂, and wisdom🧠, in other words, keeping developer community active👯👥 makes it easier to identify and solve problems that depends on the user’s/developer’s environment.

[junkmd]

@jonschz

If you would be willing to contribute to #468, I would be very happy.

[junkmd]

The commit that fixed this issue has been merged into the main branch and released with comtypes==1.3.0.