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

Clean up exports #2468

Open
penelopeysm opened this issue Jan 16, 2025 · 3 comments
Open

Clean up exports #2468

penelopeysm opened this issue Jan 16, 2025 · 3 comments

Comments

@penelopeysm
Copy link
Member

penelopeysm commented Jan 16, 2025

Turing.jl/src/Turing.jl

Lines 80 to 84 in 24d5556

export @model, # modelling
@varname,
@submodel, # Deprecated
to_submodel,
DynamicPPL,

Turing re-exports the entire DynamicPPL module, as shown above. This means that any breaking change in DynamicPPL is also technically a breaking change in Turing.

However, we haven't really obeyed this at all: many breaking changes in DynamicPPL have been released as patch level bumps in Turing. This means that technically, a patch version of Turing could break any code that looks like

using Turing
Turing.DynamicPPL.foo(...)

where foo was something that was changed in a minor release of DynamicPPL.

'But, Penny, who would write such horrible code?' I hear you ask. Well, if nobody would do that, then there's no reason to re-export DynamicPPL ;)

@penelopeysm
Copy link
Member Author

penelopeysm commented Jan 16, 2025

While we're at it, maybe we don't need to re-export everything from these packages (though I could see making an exception for Distributions.jl):

@reexport using Distributions, MCMCChains, Libtask, AbstractMCMC, Bijectors

And maybe we can re-export LinearAlgebra.I too.

@penelopeysm
Copy link
Member Author

penelopeysm commented Jan 16, 2025

Finally, I think the export list should be sorted / organised by the origin package of each symbol being exported, so the DynamicPPL stuff should be grouped, etc. It kind of is already, but not totally.

That will make it easier for us to know when something requires a breaking release.

@penelopeysm penelopeysm changed the title Stop re-exporting DynamicPPL Clean up exports Jan 16, 2025
@mhauru
Copy link
Member

mhauru commented Jan 17, 2025

Libtask I would think wouldn't need any re-exporting, it seems like an implementation detail to me. Maybe Bijectors likewise. We probably need something from AbstractMCMC and MCMCChains, but I, too, would like to be more selective.

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

No branches or pull requests

2 participants