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.

https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/tools/codegenerator.py#L809-L859

• https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/tools/codegenerator.py#L799-L807

A com object which has _NewEnum will be Iterator by metaclass and patching. https://github.com/enthought/comtypes/blob/fed3de69ccc61e37187b4a219daa120a97740663/comtypes/init.py#L410-L433

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. https://github.com/enthought/comtypes/issues/353#issuecomment-1256113572

[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.

https://github.com/enthought/comtypes/blob/6d136d1c04ac5b6479d8762ce89237351ceb66a4/comtypes/test/test_portabledevice.py#L1-L60

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;

https://github.com/enthought/comtypes/blob/6d136d1c04ac5b6479d8762ce89237351ceb66a4/comtypes/tools/codegenerator.py#L998-L1048

https://github.com/enthought/comtypes/blob/6d136d1c04ac5b6479d8762ce89237351ceb66a4/comtypes/tools/codegenerator.py#L1032 ↓

            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.