enthought
Relative imports
Hello and thanks for keeping maintaining this lib!
comtypes being pure Python, in some case it could be useful to just embed it in a larger package and be used as a COM client. However it seems it is not possible as-is mainly due to absolute imports used everywhere in the library.
Do the maintainer(s) would see a technical con with merging a patch that converts them to relative imports?
Note that I am aware of the implemented caching mechanism and the related comtypes.gen sub-package.
Hi,
However it seems it is not possible as-is mainly due to absolute imports used everywhere in the library.
Regarding the above, writing a reproducer might help us understand the specific problem.
It would be most helpful if you could provide a GitHub Actions workflow that can reproduce the problem, but manual steps are also acceptable.
TL;DR
- the ultimate point of this issue is to have comtypes' inner imports converted from absolute to relative
- example: to have
from ._npsupport import interopinstead offrom comtypes._npsupport import interopcurrently - I am willing to provide that patch by the means of a Pull Request but I want to first ensure comtypes maintainer(s) see no technical con in doing so
- considering current state of comtypes source code, this modification might not be trivial as it seems there are some hacks to avoid cyclic imports for instance, and the cache feature init code might also have to be slightly upgraded
Rationale
To elaborate on the use-case, for instance if I modify comtypes features by changing its source code (making it incompatible with your upstream version), at the moment I cannot just insert comtypes package (files) in a larger project so to test this new local and modified version of comtypes.
With most pure Python packages, doing relative inner imports instead of absolute ones solves the issue.
Granted it might not be enough in comtypes' case, due to its cache feature for instance, but it would at least be a step forward.
To illustrate:
my_app_repo/
├── my_app/
│ ├── vendor/
│ │ ├── comtypes/ # <-- modified comtypes package files copied over
│ │ │ └── ...
│ │ └── __init__.py
│ ├── __init__.py
│ └── foo.py
├── README.md
└── setup.cfg
Ideally, from my_app.foo module, I should be able to just do:
from .vendor import comtypes
Which would allows me to ensure my client code is using my locally modified and embedded version of comtypes.
But for now, I cannot do that because of the way comtypes imports its own sub-modules in an absolute fashion instead of relative.
This either forces me to modify the globally installed version of comtypes, or to trick Python's import machinery with a dirty hack at runtime.
Also:
- in regards to project's history, absolute imports in comtypes seem to be an inheritance from py2 era and from what I understand project has moved toward py3-only compliance in main branch
- moving to relative imports could also help with #867
@moi15moi why the downvote?
@moi15moi why the downvote?
If you want to copy comtypes code in your project (which is really weird) to edit it, thats is your problem. You should fix the problem that comes with it.
comtypes shouldnt support these use case because you crrate a problem where there isnt.
If you want to copy comtypes code in your project (which is really weird) to edit it, thats is your problem. You should fix the problem that comes with it.
comtypes shouldnt support these use case because you crrate a problem where there isnt.
I think you misinterpreted the purpose of this illustration? Nice touch by the way. Too bad to still have to read this kind of bold, assertive and opinion-based comments :( Have you actually ever considered relative imports for comtypes? If yes, perhaps a better way would have been to address the topic?
Also, it seems you have very strong opinions about it: what do you think is ~wrong~ weird with having to modify a Python library because it does not fully fit client code's technical constraints, then embedding it in a larger code base as long as its license is honored and preserved?
Anyway, what you actually miss here is that I am just offering to contribute back, since I am a comtypes user myself. Moving from absolute to relative imports could help with, fixing existing issues (e.g. #867, ..) and would be a step towards comtypes source code modernization.
Have you actually ever considered relative imports for comtypes?
Yes, this is clearly a plus. comtypes should use relative import because this is the new standard, but it isn't the end of the world if it doesn't.
then embedding it in a larger code base as long as its license is honored and preserved?
This is just not the way you should do it. Normally, you fork the project and edit and create wheel for it, so you can import it in your project. Like that, there is a separation between your project and your comtypes fork which, in the long term, easier for the maintenance.
Yes, this is clearly a plus. comtypes should use relative import because this is the new standard, but it isn't the end of the world if it doesn't.
This is just not the way you should do it. Normally [...], easier for the maintenance.
"comtypes should use relative import" but still, you downvote the issue and provide no technical insight. Which was the only thing I asked, a technical insight.
Plus you state (your?) general rules while forgetting about the notion of context. In the context of this issue, maintaining such a (heavy) patch locally in sync with upstream releases is not "easier for the maintenance".
What is easier here is to provide a patch in order to modernize this project after having discussed about it with the maintainers so to ensure it is done the right way and implemented properly while maintaining backward compatibility. Which I am trying to do here. Which you are not helping with. At all.
I see that the discussion in this issue is getting heated.
In any kind of communication, not just in open-source development, it's easy to have misunderstandings.
As members of an open-source community, we all need to be respectful and keep our discussions calm. To make sure we understand each other's intentions correctly, please take a moment to cool down and return to a constructive discussion.
I don't object to modernizing this project's codebase by using relative imports. However, it will be a considerably large change. While this isn't a high priority for me, I'm willing to review patches if they include sufficient tests and are submitted in a manageable size.
What I don't understand is the following part of your argument:
To elaborate on the use-case, for instance if I modify comtypes features by changing its source code (making it incompatible with your upstream version), at the moment I cannot just insert comtypes package (files) in a larger project so to test this new local and modified version of comtypes.
Why can't you solve this with an "Editable Installs" into a venv or virtualenv? https://setuptools.pypa.io/en/latest/userguide/development_mode.html
I'm not an expert on embedding Python, so I'm not sure if my question is on target or what the actual interaction would be. If this approach isn't feasible, please provide a reproducer to demonstrate why.
I don't object to modernizing this project's codebase by using relative imports. However, it will be a considerably large change. While this isn't a high priority for me, I'm willing to review patches if they include sufficient tests and are submitted in a manageable size.
I may add that while I appreciate your priorities might be different than the corner-case I describe, this issue is also a symptom that there is room for improvement. And now that py2 support has been dropped out of the main branch, some issues like #218 and #867 are easier to solve upstream without sacrificing backward compatibility (unless I am mistaken since I am no comtypes expert).
comtypes being a pure Python package with no required dependency, in theory there is no technical reason for it not to be movable as-is from a project to another. And considering the nature of its features, zip_safe for instance is a property that should also come with it for free I think.
Although I am a bit concerned about the cache feature itself as well as its auto-init part, which I believe should be configurable from client code ideally instead of current hard-coded behavior (again, unless I am mistaken), hence the unit-testing part you mentioned, definitely.
Beside the cache feature, is there any other part that I may have missed but may come into my way while converting comtypes imports?
What I don't understand is the following part of your argument:
To elaborate on the use-case, for instance if I modify comtypes features by changing its source code (making it incompatible with your upstream version), at the moment I cannot just insert comtypes package (files) in a larger project so to test this new local and modified version of comtypes.
Why can't you solve this with an "Editable Installs" into a venv or virtualenv?
What I meant is that, because of its absolute imports, my customized version of comtypes cannot just be moved around as-is and tested during its development in the desired environment - embedded or not - without extra tweaking despite it being pure Python with no dependency.
By tweaking I mean either modifying sys.path, or adding any kind of import hook, or having to create a virtual env just because of it.
If you need more details for the curiosity, in my case my_app is meant to be run by either an embedded or a vanilla interpreter (there are multiple projects that rely on my_app as a dependency).
So in the case of an embedded interpreter, your project is the environment so you are in control. That includes of course what is imported and from where.
In an non-embedded environment, a virtual env can still be created in my case but it would mean different code structure and execution path between embedded vs. non-embedded, which is a meh at best and clearly an open door to fancy bugs like behavior inconsistencies between environments.
None of this is worth the hassle when you consider it is just because of absolute imports, and that it is solvable much more easily upstream, and that it would be an improvement to comtypes anyway. Which I am offering to do.
I'd like to confirm something before we continue our discussion.
So, your customized version of comtypes only changes absolute imports to relative imports so it can be stored in a vendor directory and used in an embedded environment, and you haven't added or changed any other features?
So, your customized version of comtypes only changes absolute imports to relative imports so it can be stored in a
vendordirectory and used in an embedded environment, and you haven't added or changed any other features?
My local version of comtypes is only used as a COM client and has ended up being heavily modified.
Mostly, I converted imports to relative, fully disabled caching, modified or just tweaked some features that are specific to the running environments of my_app and that are not relevant here, added utility features that are also quite specific to the COM objects it interacts with but that are still bound to how comtypes works hence their implementation in comtypes itself.
Sorry I'm late.
To elaborate on the use-case, for instance if I modify comtypes features by changing its source code (making it incompatible with your upstream version), at the moment I cannot just insert comtypes package (files) in a larger project so to test this new local and modified version of comtypes.
That portion of the message may have led to a misunderstanding.
Is my understanding correct that you have found parts of your customized comtypes that would modernize the codebase if they were integrated upstream, and you'd like to contribute them?
I've also used a "customized comtypes" in the past.
And I have experience submitting and merging pull requests while keeping compatibility in mind.
In some cases, I didn't open a pull request for certain "customized" features because they would have broken compatibility.
So, if part of your code would help modernize this project's codebase, I'm not against merging just that part.
However, based on our discussion, it seems you've only used comtypes for COM client purposes.
This makes me think your considerations for features like caching and the comtypes.gen subpackage, which are important parts of this project, may not be sufficient.
We need to ensure compatibility if we introduce relative imports to this project.
This means not only changing the static codebases but also the dynamically generated codebases in comtypes.gen.
We can move this discussion forward if you share your vision for how to change the caching and code generation parts.
Please allow me to summarize and clarify:
- The goal of this issue is to have a patch provided and merged that converts the absolute self imports of comtypes to relative while keeping backward compatibility.
- Due to how comtypes is implemented and its history, this is a heavy and non-trivial patch that necessarily involves other modifications than just the imports themselves.
- This issue never was about providing my locally modified comtypes, nor parts of it, but about writing a patch from scratch specifically to fix that issue and only that issue.
Extra information:
- In comtypes, there are a lot of cyclic imports and internal inter-dependencies between sub-modules. Solving this would imply moving code blocks internally, and even create new sub-modules so to have very generic code moved there and reduce inter-dependency.
The questions I asked in this thread that remain unanswered:
-
I am aware of how the cache feature generates code, including the fact that it generates absolute imports too. My understanding is such a patch requires the cache feature to be modified and partly re-written. Do you think of any blocking thing that would prevent me from keeping backward compatibility along the way? Obviously if the answer is yes, this would indeed discards this patch and have this issue closed.
-
Beside the cache feature, is there any other part that I may have missed but may come into my way while converting comtypes imports?
Meta-discussion:
This venerable library helped me solving issues much quicker than if I would have had to reinvent the wheel and I just wanted to contribute back in a significant way.
Yes, this is a non-trivial and heavy chunk of a patch. It does mean things have to be re-arranged and even re-written for some parts. And I do believe it eventually should involve at a bump of major version number. That is because even in the unlikely case backward compatibility is absolutely fully preserved, such many and deep modifications may impact client code in unforeseen ways despite unit-testing, code coverage, code linting, peer reviewing, and so on...
Still I think this would be a benefit for comtypes to have its code base refreshed. I do intend to preserve backward compatibility but OTOH the technical toll of such a lengthy history of this code base is growing and may prevent such evolution. In the context of comtypes, a partial refactoring can considered a reasonable option, which this patch obviously involves.
And yes, that is a project in itself. If that is not an option for you or a path you rather not follow, I would perfectly understand and accept it of course, and leave it here.
Let me first answer the "questions [...] that remain unanswered".
- I am aware of how the cache feature generates code, including the fact that it generates absolute imports too. My understanding is such a patch requires the cache feature to be modified and partly re-written. Do you think of any blocking thing that would prevent me from keeping backward compatibility along the way? Obviously if the answer is yes, this would indeed discards this patch and have this issue closed.
It's hard to say definitively whether there are any blocking obstacles. A better way to put it is that the implementation is so complex that it's difficult to judge the full impact of the changes.
As you know, the caching feature is a collection of metaprogramming that uses importlib, sys.modules, and dynamic module generation.
It's unclear if the current behavior will be compatible with a path set in a customized gen_dir where the cache files are stored.
It might be possible to handle this just by changing the dynamically generated codebase to use relative imports, or it might not.
I think whether we can merge them will depend on what situations the tests can cover (or if the existing tests already cover them).
You might be able to gain some insight into the tests by reading test_client.py and test_client_regenerate_modules.py.
- Beside the cache feature, is there any other part that I may have missed but may come into my way while converting comtypes imports?
I believe the only technical difficulty in converting to relative imports is the caching feature. For the statically defined codebase, I think we just need to switch to relative imports.
Yes, a single patch to convert the entire package to relative imports would certainly be huge.
We are still in a situation where we're unable to gauge the full cost or unknown impact of moving to relative imports. Therefore, I cannot say for certain at this stage that your entire planned patch for relative imports will be accepted.
Please note that this is in NO way a sign that I disagree with your proposal.
However, we can make it easier to review and merge by splitting the patch into smaller, more manageable units.
For example, patches that focus on:
[…] moving code blocks internally, and even create new sub-modules so to have very generic code moved there and reduce inter-dependency.
This would still be useful even with absolute imports.
I would recommend starting with smaller changes when you first contribute to an open-source codebase you're new to. A series of small refactorings will likely lower the cost of reviewing and understanding the impact of the final functionality changes you envision.
[...] For the statically defined codebase, I think we just need to switch to relative imports.
Unfortunately for the both of us, this is a wrong guess. Because of the aforementioned inter-dependencies scattered everywhere, which do require moving quite a bit of generic code and maybe create extra sub-module(s) so to centralize this base code.
Regarding this, and this is important: I have hard time understanding why such inter-dependencies ended up being how they are right now. Lots of generic code is scattered in multiple places but still, are a dependency of modules located in a different sub-package. It does seem to have historical roots. My point is, this issue might become a blocking one on my journey to achieve this patch since it would involve too much moving or refactoring.
However, we can make it easier to review and merge by splitting the patch into smaller, more manageable units.
I would recommend starting with smaller changes when you first contribute to an open-source codebase you're new to. A series of small refactorings will likely lower the cost of reviewing and understanding the impact of the final functionality changes you envision.
Yes, this is the intended way. Practically, I will create a single branch out of main's HEAD, and make series of logical commits.
However such a patch definitely involves much bigger commits than the average commit in comtypes history. Also, as you may guess only the final commit of this new branch will have the project in a seemingly workable state. And that is if and only if the patching attempt ends up being successful from a technical standpoint.
This project is a collection of many metaprogramming layers, which makes its interdependencies more complex than a typical application codebase.
Please understand that an open-source project's codebase is not always in an ideal state, as it depends on the resources of its contributors.
Even packages far more famous and widely used than comtypes often contain code that is clearly old or irrational.
IMO, that's a success if the codebase is "better than it was before", even if there's a gap between what a contributor once envisioned and what the actual future holds.
As I mentioned before, I cannot say for certain that this issue's proposal will be accepted at this stage. If you are willing to contribute to making this package better, even with the possibility that not all of your proposals will be accepted, I welcome you to open small pull requests one by one.
@junkmd just so to let you know I could not apply relative imports and by doing so modernize the library the way I intended to without sacrificing backward compatibility at large.
As previously mentioned, biggest issue was definitely inter-dependencies between internal modules, leading to huge rewrites and heavy code restructuring, thus smashing backward compatibility way too hard.
That, mixed with the significant uncertainty that the time spent on it would eventually lead to an approved (but too complex) patch, made me more inclined to stop halfway and go for a full modern rewrite so to just fit my needs (COM client only), which are of a more humble scope than the perimeter of comtypes.
As far as I am concerned, you can close this issue. Sorry.