Fix importing of namespace packages and built-in modules #227
Conversation
eltoder
force-pushed
the
feature/namespace-packages
branch
from
e147113 to
7fe47c9
Compare
|
Thanks @eltoder for taking the time to research this and come up with an improvement. I'll have a look soon to see if we can merge this and get a new release out. I'm not sure why the mypy github action now fails. Looks like error filtering mechanism isn't working properly 🤔 |
|
@ariebovenberg I added another commit with type annotations for all tests functions. Let's see if this fixes mypy failures. I'm actually not sure how it works otherwise -- running mypy on tests fails for me locally without this change. |
|
I figured out why -- the overrides in mypy.ini don't work without the |
Ah yes—that explains. Having to add decision: let's keep the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 502 495 -7
Branches 104 103 -1
=========================================
- Hits 502 495 -7 ☔ View full report in Codecov by Sentry. |
* Namespace packages commonly don't use `_NamespaceLoader`, and it is an implementation detail in any case. The loader is then set to None. There is no need to inspect the loader anyway. Everything we need is on the `spec`. The unit test had a "work-around" for this, but this does not happen in real usage. * There are more built-in modules than just "builtins". The full list is available in `sys.builtin_module_names`[1]. However, we don't need to handle them specially. * It is also hard to know if a standard library module is built as an extension or a built-in. For example, in my Ubuntu 22.04's build of python 3.10 both `_pickle` and `_elementtree` modules are built-in[2]. Use a third-party module (`ujson`) for tests, which is guaranteed to be an extension module. * pydantic's `__init__` is no longer an extension module, so use mypyc instead, which we already have through mypy. [1] https://docs.python.org/3/library/sys.html#sys.builtin_module_names [2] https://github.com/python/cpython/blob/519b2ae22b54760475bbf62b9558d453c703f9c6/PC/config.c#L87
eltoder
force-pushed
the
feature/namespace-packages
branch
from
5abb5e4 to
30f57c0
Compare
|
@ariebovenberg Done. Also, I realized that we no longer need the special logic for builtins, and in fact should not skip the location check as we did previously. I added a test for this. |
|
version 0.18.0 is now out with this change 🚀 |
|
Thank you! I actually have another related change. I'll open a PR shortly 😅 |
Namespace packages commonly don't use
_NamespaceLoader, and it is an implementation detail in any case. The loader is then set to None. There is no need to inspect the loader anyway. Everything we need is on thespec. The unit test had a "work-around" for this, but this does not happen in real usage.There are more built-in modules than just "builtins". The full list is available in
sys.builtin_module_names[1].It is also hard to know if a standard library module is built as an extension or a built-in. For example, in my Ubuntu 22.04's build of python 3.10 both
_pickleand_elementtreemodules are built-in[2]. Use a third-party module (ujson) for tests, which is guaranteed to be an extension module.pydantic's
__init__is no longer an extension module, so use mypyc instead, which we already have through mypy.[1] https://docs.python.org/3/library/sys.html#sys.builtin_module_names
[2] https://github.com/python/cpython/blob/519b2ae22b54760475bbf62b9558d453c703f9c6/PC/config.c#L87