-
-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add enumeration definitions in the friendly modules #475
Add enumeration definitions in the friendly modules #475
Conversation
This is backward-compatibility-breaking changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion this is OK. But it should be carefully described in the change log and README.
I think the summary of this change would be worded as such.
|
In my opinion, backward-incompatible changes are okay if they're well-communicated and have value. I don't have enough insight to assess these tradeoffs. I did notice that this change is identified for a 1.3 milestone, which doesn't sound like a backward-incompatible change. Use your judgment. I would recommend not coupling this work with the drop_py2 work. |
Ok, I see where the other maintainers are concerned about the situation. I agree not to merge this PR currently yet. I would like to leave it open to let the community know that there are plans to add this feature. |
And I am considering adding a label to put on such PRs. |
close and re-open to re-run CI |
The codebase conflicts that occurred during the merging of #493 have been resolved. And CIs are passed. |
Is it still breaking backward compatibility? Can we consider supporting both new approach and deprecated legacy names? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach looks sound and valuable.
I don't have any substantive suggestions.
If I were rolling out this change, here's what I'd recommend:
- Release it as a 1.x change noting the breaking behavior.
- Provide advice to users on how to identify if the breaking behavior affects them and how to migrate.
- Be prepared to back out the change if it causes unexpected breakage that's not readily mitigated by the users.
- If backing out the change, explore a transitional approach that gives users advanced warning and a better path to migration.
Of course, we should tell that For example (but I have not seen anything like this), projects with the following codebase will not work with this change. from comtypes.gen.friendly import TgtEnum
a = TgtEnum.from_param(...) However, I believe that we have found a way to support both new approach and deprecated legacy names. For example, after releasing a version with this PR, users can still use the old definitions by changing import and the definitions of module-level symbols as follows; - from comtypes.gen.friendly import TgtEnum
+ import ctypes
+ TgtEnum = ctypes.c_int
a = TgtEnum.from_param(...) - from comtypes.gen.friendly import TgtEnum
+ from comtypes.gen import friendly
+ TgtEnum = friendly.__wrapper_module__.TgtEnum
a = TgtEnum.from_param(...) - from comtypes.gen import friendly
+ from comtypes.gen.friendly import __wrapper_module__ as friendly
a = friendly.TgtEnum.from_param(...) By including these examples in the release note, there will be less confusion for users. |
2b1a50c
to
e09469b
Compare
e09469b
to
057a1cb
Compare
057a1cb
to
7fc2aaa
Compare
7fc2aaa
to
12b2c1f
Compare
#345
I have modified the
codegenerator
so that the Python friendly enumerations are defined in the friendly module as derived classes ofenum.IntFlag
.Specifically, for example, the
Scripting.py
changes as follows before and after this change.↓
(edited: code snippet was changed after merging #493)
Previous description:
↓
So far I have been trying to define enums in the wrapper modules.
However, the executable wrapper module code is generated relying on the enumeration type names are assigned as aliases of
c_int
.Defining enumeration types in the friendly module does not affect the wrapper module implementation. And it is a low-cost implementation way.
This is backward-compatibility-breaking that changes symbols in the friendly modules previously imported as aliases of
c_int
.There may be projects that still want to use these symbols (like
TgtEnum
) as aliases forc_int
after this change.But, in that case, changing the codebase as followings are able to correspond without much effort.
from comtypes.gen.friendly import TgtEnum
from comtypes.gen import friendly; TgtEnum = friendly.__wrapper_module__.TgtEnum
TgtEnum = c_int
(edited: explanation of alternatives was changed after merging #493)
Previous description:
There may be projects that still want to use these symbols as aliases for
c_int
after this change.But, in that case, changing the code from
from comtypes.gen.friendly import TgtEnum
toTgtEnum = c_int
is able to correspond without much effort.