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

Rework namespace extension #218

Closed
FlorianWilhelm opened this issue Sep 9, 2018 · 11 comments
Closed

Rework namespace extension #218

FlorianWilhelm opened this issue Sep 9, 2018 · 11 comments
Labels
enhancement New feature or request
Milestone

Comments

@FlorianWilhelm
Copy link
Member

Right now we historically use pkg-resource-style namespace packages. According to the official packaging guide this seems to be still valid but an outdated option. With version 4.0 we should break with this behavior and should start using the new native namespace packages as define in PEP 420 that requires Python 3.3 and above.

@FlorianWilhelm FlorianWilhelm added the enhancement New feature or request label Sep 9, 2018
@FlorianWilhelm FlorianWilhelm added this to the v4.0 milestone Sep 9, 2018
@FlorianWilhelm
Copy link
Member Author

@abravalheri, I am not sure anymore if we should really change this. I see no advantages except of the fact that they call it recommended in the docs. But there are risks of course as always. Do you see any advantages at all? Ah, and the fact that find_packages doesn't work anymore seems to indicate that we would have to list all packages explicitly in setup.cfg again. I find our current solution with

[options.packages.find]
where = src
exclude =
    tests

much nicer.

@abravalheri
Copy link
Collaborator

Yeah, I was reading the links you sent this morning and wondering the exactly same thing...

Is pkgutil-style namespace packages better? Does it allow us to use the same current approach?

@FlorianWilhelm
Copy link
Member Author

@abravalheri Okay, then I think that we agree on the fact that the new native namespace packages seems to give us no tangible advantages but a lot of risks and they need more configuration in setup.cfg. Consequently, we shouldn't introduce them in PyScaffold 4.0 just for the sake of having a new feature. Also it says in the packaging guide that one should stay with pkg-resources-style namespace packages and not migrate to something new. Also the pkgutil-style namespace packages seem to provide no advantages. So, let's stick to what we have :-)

@abravalheri
Copy link
Collaborator

So it seems that setuptools now provides a find_namespace option which implements PEP 420 and can be used from setup.cfg. Moreover, it is backward compatible (this is the wording used in the docs and the implementation is done via inheritance), which means it could be used as more powerful a drop in replacement for find (it might be misleading though when no namespace package is used).

The main disadvantage in using it instead of find is that it would normally end up classifying all the folders in a repository as namespace packages, but since we use a src-layout we probably are "imune" to that... (the best is to run some experiments).

I will re-open this issue, for investigation (specially because obviating pkg_resources is a milestone in setuptools, see #309)

@abravalheri abravalheri reopened this Aug 7, 2020
@smartsammler
Copy link
Contributor

Since setuptools version 40.1 there is the find_namespace: directive with which it's probably possible to create native namespaces (PEP420).
We tested a namespace package setup with PyScaffold, deleted the src/$namespace/__init__.py and changed packages = find: in the setup.cfg to packages = find_namespace:, increase the required setuptools version to 0.40.1 and it worked.
We did so, because we wanted to build Debian packages with py2dsc-deb and the installation failed/needed extra arguments, because the __init__.py was not replaced automatically.

@abravalheri
Copy link
Collaborator

Thank you very much @smartsammler! Nice to know that this works, it will probably be part of v4.

Now the only thing I am not exactly sure is if we should always use find_namespace instead of find (since it does not seem to be dangerous for src-based layouts) or if we do it conditionally, just in case the user activates the --namespace extension.

@smartsammler
Copy link
Contributor

Actually Sphinx did not work anymore correctly. To fix this one also has to provide the --implicit-namespaces flag to sphinx-docs.
docs/conf.py:

-    cmd_line_template = "sphinx-apidoc -f -o {outputdir} {moduledir}"
+    cmd_line_template = "sphinx-apidoc --implicit-namespaces -f -o {outputdir} {moduledir}"

(Maybe this information is helpful in case someone also want to use putup and then change to native namespaces).

Unfortunately I don't know if find_namespace does no harm to non-native namespace packges, nor do I know it for the Sphinx configuration.

@abravalheri
Copy link
Collaborator

Thank you very much @smartsammler, I think the best is to do the change you propose, and see if we have any feedback, but I suppose as long as we are using the src-layout we should be good to go (everything inside the src folder should be part of the package).

@smartsammler
Copy link
Contributor

@abravalheri So one really has to be aware of the implication that those two namespacing concepts aren't compatible with each other. So one has to update all projects once using the implicit namespaces, as far as I understood and experienced this.
So if @FlorianWilhelm aren't sure about including it in 4.0, it might be an option to create an extension that provides implicit namespaces instead of replacing the namespace internal extension. --namespaces (pkg-resources) vs. --implicit-namespaces (PEP420).

Some colleagues of mine for example had examples in src/examples.py which afterwards did not work anymore, because they couldn't import from the source tree, but had to import an installed version, so it might break workflows. However, 4.0 is a breaking release anyhow.

@abravalheri
Copy link
Collaborator

Hi @smartsammler, I have provided a PR (#328) with the suggested fix and a bunch of new documentation on the topic. Please feel free to review it.

My opinion is that we should move forward by being opinionated, but still providing documentation for people that want to preserve the old behaviour. Since packages = find_namespace: solves most of our problems and actually simplify stuff, I think we should attempt to comply with the new officially blessed practice, and avoid being taken by surprise if pkg_resources is abandoned some day (for all effects we can already consider it deprecated). Since we are working in a major version release, we are in the best moment to introduce non backwards compatible changes.

I understand that some people may have particular use cases and I tried to cover as much as I could in the additional documentation in the PR. I hope that is enough.

@abravalheri
Copy link
Collaborator

For the time being I will merge the PR, since there is a bug with the current v4.0.x branch and the PR fix it...
Of course we can change the overall design later and allow "opt-in" for implicit namespaces...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants