Skip to content

makepy: support for enums and type-hints - next try#1909

Closed
tobias-loew wants to merge 0 commit intomhammond:mainfrom
tobias-loew:main
Closed

makepy: support for enums and type-hints - next try#1909
tobias-loew wants to merge 0 commit intomhammond:mainfrom
tobias-loew:main

Conversation

@tobias-loew
Copy link
Copy Markdown

Hi,

I adopted my changes from #1364 to the new main branch.

So, let me come back to the open questions from #1364 :

What's an enum in this context?
It's a IDL enum (i.e. a C++ unscoped enum) on the COM-side (https://docs.microsoft.com/en-us/windows/win32/midl/enum). And for Python it's either a native python-enum (https://docs.python.org/3/library/enum.html) or it's a "relaxed" python enum that adds unknown values on the fly (see implementation in RelaxedIntEnum.py)

Type-hints are defined here https://peps.python.org/pep-0484/ and come in very handy when working with large libraries

The NativeTypeMap is needed to generate the appropriate type-hints for COM-automation types

some words on iCreateEnums:

Python-enums started with version 3.4, but versions before have to be supported, so enum-generation is an opt-in (same for type-hints). That's also the reason I didn't use an enum for iCreateEnums.
Why the hell are there two different enum-generation options (apart from "no enums" iCreateEnums == 0)?
option iCreateEnums == 1 generates native Python enums. But there is an issue: a COM method may be called with a value for an enum that is not among the "known" values. The Python-enum will throw an exception in that case. Which may be good or not. It depends on, what the user expects. So, there is also
option iCreateEnums == 2, which creates "RelaxedIntEnum"s which simply add unknown values to their internal map and don't throw.

The CLI currently only allows to select option 2, but this could be changed easily.

Tobias

@mhammond
Copy link
Copy Markdown
Owner

Thanks for this! I've had lots of time chewed up tracking down #1912 and I'm traveling for work over the next few weeks, so it might be a little time before I get back to this, but I will!

@mhammond
Copy link
Copy Markdown
Owner

Thanks heaps for pushing on this!

What's an enum in this context? It's a IDL enum (i.e. a C++ unscoped enum) on the COM-side (https://docs.microsoft.com/en-us/windows/win32/midl/enum). And for Python it's either a native python-enum (https://docs.python.org/3/library/enum.html) or it's a "relaxed" python enum that adds unknown values on the fly (see implementation in RelaxedIntEnum.py)

There are a couple of test enums here and they already have tests - any chance of new tests to exercise the enum part of this?

And can you please explain a little more about the Relaxed enum - that makes sense to me for "dynamic" objects, but less for makepy generated support, but I think they are used by makepy?

Type-hints are defined here https://peps.python.org/pep-0484/ and come in very handy when working with large libraries

Is there a reason to not unconditionally generate these type-hints? It would make the patch simpler and can't see a compelling reason to make this optional?

Python-enums started with version 3.4, but versions before have to be supported,

Not any more - pywin32 only needs to support 3.6+, so I'd be in favour of making this non-optional too - but I haven't looked at this closely enough to understand if there are b/w compat issues - which tests would also help me understand.

The CLI currently only allows to select option 2, but this could be changed easily.

The less options the better IMO :)

@tobias-loew
Copy link
Copy Markdown
Author

There are a couple of test enums here and they already have tests - any chance of new tests to exercise the enum part of this?

I'll have a look at it

And can you please explain a little more about the Relaxed enum - that makes sense to me for "dynamic" objects, but less for makepy generated support, but I think they are used by makepy?

The problem is that many COM-Servers still evolve. Take for example Excel: every version introduces some new constants to the existing enumerations (e.g. https://docs.microsoft.com/en-us/office/vba/api/excel.xlfileformat). Thus, a python client that was build with an Excel 2013 COM-Lib and is run today against a newer version, may receive values for an enum that haven't been available back at build time.
A standard python-enum would then raise an exception, but the "non-enum" pywin32-client would have simply passed on an integer. The RelaxedEnum simply ensures, that the pywin32 client doesn't throw exceptions when it encounters unknown enum values.

Is there a reason to not unconditionally generate these type-hints? It would make the patch simpler and can't see a compelling reason to make this optional?

Just because they required 3.5. I'll make it unconditionally.

The less options the better IMO :)

Totally agree

@mhammond
Copy link
Copy Markdown
Owner

I'll have a look at it

Thanks!

The problem is that many COM-Servers still evolve. Take for example Excel: every version introduces some new constants to the existing enumerations

I don't find that hugely compelling TBH - aren't they also going to want the new functions etc in the new version. IOW, why don't they just upgrade?

(I'm not totally against it, just don't find it compelling :) #1931 is basically doing the same thing for other attributes - I'm not sure the motivation there is that an old version of the typelib was used, but I guess the end result is the same)

I'll think a little more about it while you make the changes you mentioned.

@tobias-loew
Copy link
Copy Markdown
Author

The problem is that many COM-Servers still evolve. Take for example Excel: every version introduces some new constants to the existing enumerations

I don't find that hugely compelling TBH - aren't they also going to want the new functions etc in the new version. IOW, why don't they just upgrade?

(I'm not totally against it, just don't find it compelling :) #1931 is basically doing the same thing for other attributes - I'm not sure the motivation there is that an old version of the typelib was used, but I guess the end result is the same)

I'll think a little more about it while you make the changes you mentioned.

I have the following scenario in mind:

  1. someone writes an com-client in python with enum-support, e.g. for using Excel
  2. somebody else just uses that library against a newer Excel-Version than the one used to build the library. If some call to Excel returned a new introduced enum value (like e.g. a new file-format-identifier) or an undocumented value, than a standard python-enum would throw an exception (regardless of whether the value was used or not).

I'm not sure, if users would expect that behavior. Especially, when a non-enum-using version of the library would not throw and simply pass on the value.

@tobias-loew
Copy link
Copy Markdown
Author

tobias-loew commented Sep 17, 2022

I think there is a difference to #1931:

@vladexl
Copy link
Copy Markdown

vladexl commented May 3, 2023

Crash with typehint: (it seems for structs may be)

  [propget, id(8), helpstring("Eigenschaft TableEntry"), helpcontext(223)]
  HRESULT TableEntry([in] long nIndex, [out, retval] struct ValueRangeEntry* pVal);

def _MakeTypeHint(doc, vt): <- 'doc' can be None

at this moment:
MapEntry(dispid=8, desc=PyFUNCDESC(memid=8, scodeArray=(), args=((3, 1, None, None, None, -1),), funckind=4, invkind=2, callconv=4, cParamsOpt=0, oVft=168, rettype=(36, 0, None, None, None, 1), wFuncFlags=0), names=('TableEntry', 'nIndex'), doc=('TableEntry', 'Eigenschaft TableEntry', 223, 'ComHelp.chm'), resultCLSID=None, resultDocumentation=None, wasProperty=1, hidden=False

@vladexl
Copy link
Copy Markdown

vladexl commented May 3, 2023

Another point:
it would be nice do not remove 'class constants', because of previously generated code and generate both: enums + class constants

@tobias-loew
Copy link
Copy Markdown
Author

Another point: it would be nice do not remove 'class constants', because of previously generated code and generate both: enums + class constants

It's fix now. Thanks for the error-report.

Additionally, I changed the "-e" option: it now expects an argument-list which can contain constants and classes. If both are specified (-e constants,classes) then both enum-classes and the int constants are generated

@vladexl
Copy link
Copy Markdown

vladexl commented Jun 29, 2023

New issue about type hints:
COM library can have cross references between COM interfaces, so forward declaration of COM interfaces is mandatory thing here. Otherwise you can get error like this: NameError: name 'IAttribute' is not defined

@Avasam
Copy link
Copy Markdown
Collaborator

Avasam commented Nov 3, 2023

@tobias-loew I think you accidentally changed line endings in preconn.cpp, genpy.py and makepy.py. Making for much bigger diffs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants