comtypes
comtypes copied to clipboard
enthought
Require explicit enabling of numpy interop
This PR addresses issue #333, "Delayed loading of npsupport". It does not attempt to load numpy as a library at import time, but only when explicitly requested via the new function comtypes.npsupport.enable_numpy_interop() (or by entering the existing context manager safearray_as_ndarray). This allows users to control whether (and when) numpy is imported, even if it is installed in their environment.
The current version passes all the previous tests, and has gained one checking that trying to access numpy via npsupport fails until enable_numpy_interop() has been called, but probably some further tests are appropriate to test that the right errors are raised at the right times when the function has not been called, so that any users who will be affected by this change get the most helpful error messages. However, best practice testing of this feature may require upgrading of the test environment to allow testing different python envs with and without numpy. There is also the possibility that all numpy interop could be replaced with the PEP3118 Buffer Protocol, removing all numpy related code from comtypes.
One more general thing is that only the first import of numpy is slow. All next imports are fast so sometimes maybe it's easier to do import numpy instead of calling any function.
@vasily-v-ryabov you are absolutely right, in my new version I just import numpy whenever I need it, but inside the Interop.numpy property - this is important because it lets us raise an error if users try to access it without enabling support first.
Please let me know if anything else is planned for 1.2.0 release. If that's it, I will hopefully publish 1.2.0 next week.
@vasily-v-ryabov - sorry, but I don't consider this PR ready to be merged yet - I am still working on renaming things so that comtypes.npsupport is actually the comtypes._npsupport.interop instance (there are some challenges with testing as we have to un-enable the interop between tests), drop the legacy numpy support and there is also a 1 line bug for some older versions of numpy that I found. Please can you give me a couple of hours to get those things sorted out and push a new version?
@bennyrowland no problem. I have to switch to my main job and will be able to publish 1.2.0 next week anyway. So you have enough time to create one more PR. Thanks for your efforts!
@vasily-v-ryabov, thanks. I have one final question for now - how would you feel about adding the parameterized package as a test dependency - basically it allows me to write 1 test and have it be expanded into two tests at test time, one where npsupport should be enabled, and one where it shouldn't. This makes it easy to test that the correct Exceptions get raised when interop is disabled, using the same code that is testing correct behaviour when it is enabled, without duplicating all the tests. If you are reluctant to add a dependency just let me know and I will arrange things without the package.
@bennyrowland we're trying to keep comtypes itself as pure Python package. Test dependencies are not so strict, but if it is only 1-2 tests I would think about new dependency twice. If it saves a lot of code, let's use it.
@vasily-v-ryabov, I do understand, and I figured out how to achieve the same effect with my own test decorator. If you have a look at the new PR, you can see how the tests are set up, but it does feel a bit hacky. If you don't like the way it works at the moment then I also considered setting up the decorator to run the "enabled" and "disabled" tests sequentially within the same test. That is slightly cleaner because it doesn't rely on messing about with frame locals, but it has the disadvantage that you only get one test rather than two. Also, let's move discussion on to the new PR now.