JuliaManifolds / Manifolds.jl

Manifolds.jl provides a library of manifolds aiming for an easy-to-use and fast implementation.
https://juliamanifolds.github.io/Manifolds.jl
MIT License
368 stars 53 forks source link

Consider making RecursiveArrayTools and StaticArrays weak dependencies #656

Closed kellertuer closed 2 weeks ago

kellertuer commented 11 months ago

It might be a bit counter-intuitive to the current structure, where we have these methods with specific manifolds, but I think for the package it would be neat if these packages that introduce speedups for certain data types would be weak dependencies.

kellertuer commented 9 months ago

I will maybe try this next, though for RecursiveArrayPartition this might be breaking, since previously we exported ArrayPartition. The rest seems mainly to be reorganising code.

edit: but besides that there is even an interplay between StaticArrays and RecursiveArrayTools (double-ext-necessary) Array is much less work. Will try that tomorrow then and see how much this affects other stuff.

Still this might be considered breaking.

mateuszbaran commented 9 months ago

This may indeed be a bit breaking so I'd prefer to have it scheduled for the next breaking release instead of rushing it now.

kellertuer commented 9 months ago

I started and noticed it is mainly code-reorganisation, but there might be 2-3 small breaking things

So – I will slowly do this, open a PR, but that should really only be merged when we have more reasons for a breaking change – since this decoupling will also not have such a large effect on loading and such.

So I fully agree with your opinion :)

mateuszbaran commented 2 weeks ago

We can probably close this because RecursiveArrayTools is now a weak dependency and StaticArrays is not worth extracting at the moment?

kellertuer commented 2 weeks ago

Yes.