Skip to content
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

Correct implementation of pkgutil style namespace (Fixes #144) #145

Merged
merged 2 commits into from
Jul 25, 2018

Conversation

Gadgetoid
Copy link
Collaborator

Change to correct use of pkgutil style namespace packages as documented https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages and illustrated in https://github.com/pypa/sample-namespace-packages/tree/master/pkgutil

See #144 for a longform attempt at explaining this change. The short explanation is that this package confuses the two different methods of implementing namespace packages and this change corrects the implementation to the (Python 2.3+ and Python 3.x compatible) pkgutil style method.

Note: at the moment the namespace_packages directive is completely ignored, if you add the "luma" package to packages (in addition to namespace_packages) then setup.py build returns an error because it is expecting a pkg_resources style namespace package:

running build
running build_py
error: Namespace package problem: luma is a namespace package, but its
__init__.py does not call declare_namespace()! Please fix it.
(See the setuptools manual under "Namespace Packages" for details.)

Note: in the linked example the find_packages() method returns the "namespace" package as if it were an ordinary package.

Change to correct use of pkgutil style namespace packages as documented https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages and illustrated in https://github.com/pypa/sample-namespace-packages/tree/master/pkgutil

Note: at the moment the `namespace_packages` directive is completely ignored, if you add the "luma" package to `packages` (in addition to `namespace_packages`) then `setup.py build` returns an error because it is expecting a pkg_resources style namespace package:

```
running build
running build_py
error: Namespace package problem: luma is a namespace package, but its
__init__.py does not call declare_namespace()! Please fix it.
(See the setuptools manual under "Namespace Packages" for details.)
```

Note: in the linked example the `find_packages()` method returns the "namespace" package as if it were an ordinary package.
Gadgetoid added a commit to Gadgetoid/luma.oled that referenced this pull request Jul 25, 2018
See corresponding PR to luma.core - rm-hull/luma.core#145
Original issue and explanation - rm-hull/luma.core#144
Gadgetoid added a commit to Gadgetoid/luma.led_matrix that referenced this pull request Jul 25, 2018
See corresponding PR to luma.core - rm-hull/luma.core#145
Original issue and explanation - rm-hull/luma.core#144
@rm-hull rm-hull merged commit 120df29 into rm-hull:master Jul 25, 2018
rm-hull pushed a commit to rm-hull/luma.led_matrix that referenced this pull request Jul 25, 2018
* Correct implementation of pkgutil style namespace

See corresponding PR to luma.core - rm-hull/luma.core#145
Original issue and explanation - rm-hull/luma.core#144

* Update CONTRIBUTING.rst
@Gadgetoid Gadgetoid deleted the patch-1 branch July 26, 2018 10:14
@Gadgetoid
Copy link
Collaborator Author

Based on my sleuthing here rm-hull/luma.oled#215 the following solution may be optimal since it prevents luma.oled from removing luma.core's __init__.py when it is uninstalled.

https://github.com/rm-hull/luma.core/compare/master...Gadgetoid:patch-1?expand=1

I'm now attempting to run the tests from: https://github.com/pypa/sample-namespace-packages

rm-hull pushed a commit to rm-hull/luma.oled that referenced this pull request Sep 7, 2018
* Correct implementation of pkgutil style namespace

See corresponding PR to luma.core - rm-hull/luma.core#145
Original issue and explanation - rm-hull/luma.core#144

* Update CONTRIBUTING.rst

* Update setup.py
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.

2 participants