-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Proposal: Extension-like Subpackages #58051
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
base: master
Are you sure you want to change the base?
Conversation
Does loading a submodule automatically load the main module? I think that's hinted at by
but I'm asking for clarification. My understanding is that submodules in python don't load the main package and I quite like it (if my interpretation of the behavior is correct). |
So yes, my thinking was that it should automatically load the main module. But now that you mention it, there's no real reason it has to be that way. It desired, these probably could be independently loadable. The submodule could just choose whether or not it loads the main module by |
I really dislike the naming here. We already have submodules: any module within another module might be called a submodule in relation to it's parent or some other ancestor. Introducing something else called "submodule" would be confusing. |
Sure, they can be called whatever. I'm not married to any particular name. Suggestions welcome. The reason I went with this name is that I wanted them to behave as closely as possible to the behaviour of using regular modules inside of a package, except that they're optionally loaded rather than eagerly loaded. But I agree it's not a perfect name. |
I wrote some stuff about this feature along time ago in https://hackmd.io/jpsiIxdsSRe4ANi55_F7oA (where I called it subpackages). I'll just copy and paste the pertinent questions I had in there with some comments after in parenthesis so they can be discussed here:
|
Ah thanks for sharing that hackmd, I must have missed it in the previous conversations, though I saw people discussing the questions you posed. So I think most of these questions basically boil down to "how separable should a subpackage be from the parentpackage?" I think if one answers "yes" to most of these questions, then the mechanism I'm looking at in this PR should not be used, and we should instead base this off
Yeah, I think if we want those things, then maybe this feature should be built in a different way.
Hm, interesting I hadn't really thought of needing to do that. I think we could do this with the design I have here, but it'd probably be more awkward than it having its own
Yes, this should absolutely be supported.
Not in this design.
Hm, I hadn't thought of this. I'd ideally like to share an
Yeah, Mose brought this up above. I think this can/should be done, I just didn't think to support it, but it shouldn't be too hard.
I'd say you should just add the subpackage deps as test deps, and then you should be able to just load the subpackages as needed.
In this proposed design, you don't have to, as they are not input as individual packages. My motivation for doing the PR this way, where I'm answering 'no' to a lot of the above questions was
If people feel we really do need the flexibility / clarity that comes with each subpackage having its own Project.toml, and it's own entire directory structure and all that stuff, then this PR probably isn't the right direction, but I wanted to at least try out a 'lighter', more unified structure, and see if it can be made to work. |
I've switched the PR to call them I've also edited the original post to reflect that. |
One thing that I have been missing with package extensions is the notion of them being able to have additional dependencies (I kinda understand why that is the case, but I am still sad about it) There would be a nice symmetry if a package extension is a subpackage that is automatically loaded when a trigger or trigger package is loaded. Maybe that would also solve the issue with folks wanting to export names from package extensions since you could use "using Oceanaigans::Plotting" to load the |
I guess one thing that could be done there is just put all the plotting stuff in the module OceanigangsPlottingExt
using Oceanigangs::Plotting
end # module The name = "Oceanigans"
uuid = ".."
[weakdeps]
Makie = "..."
[extensions]
OceanigansPlottingExt = "Makie"
[subpackages]
Plotting = ["Oceanigans", "Makie"] |
Yes, but you would somehow need to add an dependency arrow from the extension to the subpackage. And that is currently not possible. |
Ah I see. Yeah, we'd need to generalize the trigger mechanism a bit for that. |
So if we might want more complex trigger mechanisms for extensions anyways, then this actually brings me back to wondering if the right solution here is to just make a way to list an extension as a dependency, and make extensions Alternatively, you could instead just put the glue methods in the |
FYI. I believe at somepoint the discussion regarding to local module loading involved similar idea of this as subpackages. Except that was combined with a syntax convention as well See also #49155 I personally feel relying on configure files heavily is an overkill while the information can be brought into the source code via syntax - it is quite hard for users not familiar with all these mechenism figuring out where that module is coming from, we already suffering from the |
That discussion seems to be about something else. The core issue here is optional loading. i.e. the module We need these subpackages to have their own sets of dependencies that can be wider than the dependencies of the parent package, so that's why the config files need to be a part of it. Put another way: the goal here isn't to replace This might end up being why this PR's direction gets rejected. It tries it's best to minimize the amount of mucking about in config files and extra nested directories, but we might end up needing even more of them, and have each subpackage be a full on environment with its own |
This is exactly why I proposed it. I am not proposing to replace include. Lacking automated lazy+local loading has been a nightmare in Bloqade which is pretty much the whole story why we have to abandon the package and adopt Python in the pipeline. In interpreter languages this is "just" local module loading when interpreter see the request of loading a module which as a result to be lazy. The reason of having it instead of |
Maybe let me elarboate a bit more on what I was trying to say here. I'm not againsting this RFC. I'm proposing to use the following syntax instead of a confg file to solve the exact same problem. Tbh you already solve most of the issue here, all I'm saying is we can combine it with local module loading - this feature implemented in this PR is pretty much what local module loading means in an interpreted language except the syntax module Foo # incomplete module declaration triggers the compiler to look for a subpackage or pkgimage The benefit is you will be able to use module Moo
module Foo # optionally loads the subpackage Foo when you type `Moo.Foo`
end the And when you change your mind to move this out of the package While in the case of using a config file, if you change your mind you have to dig into the config file and update the config file and update those strange I realize the syntax is probably not the best because |
I guess I'm confused then. If you don't want config files involved, how would your preferred syntax for this handle stuff like optional dependancies? And how would a downstream package depend on some of the subpackages, but not all of them? This is important so that the environment knows what packages to precompile, and what deps to install, rather than unnecessarily precompiling all the subpackages and all the deps. |
In my opinion there are two things here:
For a. this is address by you can see similar separation in rust or Python (well not a compiler tho). I will use rust as an example here:
The reason of separation here is to make compilation less coupled with package loading. So for example, when I have a local packages/scripts, this still works well together without a package manager. For example, this is more generic because
That said, I'm not against the RFC, I'm just saying you can solve the first problem via a syntax delcaration. Yeah necessary external dependencies needs to be delcared in a config file there is no other options. But for local dependencies you don't have to use config file, but instead you can use a syntax. |
We're writing some opinionated extensions to the FunSQL ecosystem, for example, an IDE using Pluto, let's call it "PlutoFunSQL".
For now, I think we're going to have a separate mega-package, PlutoFunSQL which is opinionated and has lots of dependencies, such as Makie. The the core FunSQL keeps almost no dependencies. Once something like this is released, we could refactor, deprecate PlutoFunSQL and break this mega package into N subpackages. Note: we will use the extension feature for database drivers, but this is a separate user experience that seems quite different than this. |
This is a WIP proposal for trying to solve #55516. @KristofferC mentioned another design here that is potentially simpler, but I was curious about exploring this idea and seeing where it goes.
The high level idea here is that
[subpackages]
are very similar to[extensions]
(and hence most of the code is shamelessly stolen and adapted from Kristoffer's extensions PR). They are sub-packages that live inside of a parent package, and are not loaded by default. They differ from extensions in thatusing
-ed.Manifest.toml
.A
Project.toml
with[subpackages]
might look something likeThis says that
SomePackage.Subpackage1
depends on Somepackage.jl, and Example.jl and those must be loadable in order forSomePackage.Subpackage1
to be loaded. WhereasSubpackage3
has no dependancies and can always be loaded.There should then exist files at
SomePackage/sub/Subpackage1.jl
,SomePackage/sub/Subpackage2.jl
,SomePackage/sub/Subpackage3.jl
which define modules with those names, and they're allowed to depend on anything fromSomePackage
's deps, and whatever weakdeps they declare. They are allowed toexport
names they define.The
Manifest.toml
would look something likeI added a stopgap macro
@subpackage_using SomePackage.Subpackage2
that allowsSubpackage2
to be loaded, but this should be replaced with dedicated syntax.See the tests I added for an example of this in action.
Edits: