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
366 stars 52 forks source link

Rename Symplectic to SymplecticMatrices? #701

Closed kellertuer closed 1 day ago

kellertuer commented 7 months ago

Currently we usually suffix Matrices for quite a few manifolds where the name itself is not specific enough, e.g. SymmetricMatrices since we use Symmetric for the single matrix.

This is a bit inconsistent to one other case we have, which I therefore would like to rename

I think this would even only be mildly breaking, only the constructors need to be deprecated but could still keep working for a while. Breaking would be if other use these types, since we could not const them when reusing them. But it would be nice to have this consistently with LinearAlgebra in the naming

What are opinions on this? I am reworking a bit of this in #700, so I could even include it in there is the breaking part is considered fine enough.

mateuszbaran commented 7 months ago

I think this renaming makes sense. We can schedule it for the next breaking release.

kellertuer commented 7 months ago

Cool. I will carefully add the functionality I need on #700, and will maybe already add a few notes what might change in the future as comments

kellertuer commented 7 months ago

If we just deprecate the constructors, the only breaking part would be if someone dispatches on the old Symplectic type, the others could be kept as deprecated const definitions even. Would that still be too bad to be considered breaking? I could otherwise include this in #700.

mateuszbaran commented 7 months ago

I think we can do the first two points in #700 with deprecation and aliases but the third one IMO needs to wait for a breaking release.

kellertuer commented 7 months ago

That sounds fair enough. I will do those two then next in that PR.