flatsurf / sage-flatsurf

Flat surfaces in Sage
https://flatsurf.github.io/sage-flatsurf/
GNU General Public License v2.0
10 stars 10 forks source link

Replace the SimilaritySurface hierarchy with categories #220

Closed saraedum closed 1 year ago

saraedum commented 1 year ago

Fixes #132 and hopefully gets rid of all of the boilerplate that is currently necessary to expose functionality in both surface hierarchies.

Checklist

Dependencies
TODO
Fix Speed Regressions

Generically, some tests deteriorated:

There are specific problems with performance:

saraedum commented 1 year ago

@videlec this one is essenially ready. I need to make the documentation build again and fix some upstream issues but otherwise this is it.

I know this patch-bomb is not reviewable. I'd recommend to have a look at the news file to get a rough overview of what happened here: https://github.com/flatsurf/sage-flatsurf/pull/220/files#diff-a44f00f4a0a1ee8b0d4e68e5609335e17720b1d9b2ff039f6c74ce82cdcf5daa

The category setup is not incredibly elegant at times, e.g., to add something to oriented finite type half-translation surfaces without boundary, you have to put it in:

class HalfTranslationSurfaces:
    class Oriented:
        class FiniteType:
            class WithoutBoundary:
                class ParentMethods:
                    def stratum(self): …

We could flatten this somewhat (and work around how this is meant to be used by SageMath) but maybe this is good enough for the first iteration?


Admittedly this went a bit too far maybe. The old sage-flatsurf had 40k LoC and this changeset contains +20k/-10k; it adds a lot of documentation actually ;)

github-actions[bot] commented 1 year ago

Documentation preview for this PR is ready! :tada: Built with commit: 03eef30ed60f44fa72360e9579b6a7c6988cc3ac