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

Implement implicit namespace packages #453

Closed
wants to merge 2 commits into from

Conversation

martenlienen
Copy link

@martenlienen martenlienen commented Oct 13, 2021

Since the implementation of PEP621 in flit, we can get namespace packages basically for free. The only required change is not reading or importing the module, when the information (description and version) has already been provided in pyproject.toml. This is accomplished with just one if. The other places already avoid reading the file when project.dynamic is empty in pyproject.toml. During sdist we also have to just not check for the module file.

Best,
Marten

@martenlienen martenlienen changed the title Implement namespace packages Implement implicit namespace packages Oct 14, 2021
@martenlienen
Copy link
Author

I hope that the simplicity of the changes can lighten your concerns about the maintenance burden of this feature in #159 @takluyver. I can add additional test cases, if there are important cases that I did not cover.

@martenlienen
Copy link
Author

I have used this PR successfully today via

[build-system]
requires = ["flit_core @ git+https://github.com/martenlienen/flit.git@namespace-packages#subdirectory=flit_core"]

to release hydra-experiment-sweeper, a plugin for hydra that needs to be packaged as a namespace package.

@takluyver
Copy link
Member

Thanks @martenlienen, and sorry it's taken me a while to look at this.

Truth be told, I like the old approach of #159 better, where you package something in the namespace, rather than the namespace itself. I.e. you would specify:

[tool.flit.module]
name = "hydra_plugins.experiment_sweeper_plugin"

And then it would work like any other Flit package - with support for dynamic version & description fields extracted from the package itself. I'm not be a big fan of namespace packages myself, but if we're going to support them, I'd rather do so properly than as a half feature.

@martenlienen
Copy link
Author

martenlienen commented Nov 11, 2021

Thanks for getting back to namespace packages after all. I agree that the other approach feels nicer. The obvious advantage of my PR is just that it is such a small change. Either way, I am looking forward to having namespace support in flit eventually!

@takluyver
Copy link
Member

Thanks! A small diff is certainly nice, but I'd rather not have to explain that certain features aren't available when you're packaging something in a namespace package. 🙂

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