Some experiments about structuring the code better

[unhyperbolic]

I found it hard with all the monkey patching to actually find where a method is implemented. This is an attempt to fix it.

Let me know what you think.

[unhyperbolic]

This still causes some test failures and I have some naming questions.

The idea is that the user-facing classes Triangulation[HP] and Manifold[HP] subclass from 1. a class defined in python that brings in all the python methods and 2. a cython class bringing in the cython wrapped methods.

1. Is called Triangulation/ManifoldMixIn. I am somewhat happy with the name, but am open to suggestions. I wonder whether something like Triangulation/ManifoldPyBase are better in that they reflect the intention that this is where you add the python methods shared between Triangulation[HP] and Manifold[HP].
2. Is called Triangulation and Manifold in the cython module. I find it actually somewhat confusing that they have the same name as the user-facing classes. Using the fully qualified name such as extensions.SnapPy.Triangulation makes it a bit better. But I wonder whether they should just be called _Triangulation and _Manifold in cython or Triangulation/ManifoldCyBase to reflect the intention better.

On a related note: tests in extensions.SnapPy test against the cython base class and not the user-facing class. I would prefer to test against the user-facing class. This affects things like Manifold.is_isometric_to which we expect to work when mixing Manifold and ManifoldHP.

My first though was that after renaming, say, Triangulation in extensions.SnapPy, we replace extensions.SnapPy._triangulation_class = Triangulation by extensions.SnapPy.Triangulation = Triangulation. Then setting extensions.SnapPyHP.Triangulation = TriangulationHP would allow both the cython code to call Triangulation(...) to generate the respective python class and would work in the tests. But it feels wrong to me.

So I think we still stick with extensions.SnapPy._triangulation_class but when testing extensions.SnapPy set the globals to use the python super classes Triangulation[HP] and Manifold[HP].

Let me know whether you agree with my thoughts and which of the above naming choices you prefer.

[culler]

My initial reaction is that it would be better to have python classes named TriangulationBase and ManifoldBase and Cython classes named ManifoldKernel and TriangulationKernel. Then Triangulation could inherit from TriangulationBase and TriangulationLKernel, while Manifold could inherit from Triangulation, ManifoldBase and ManifoldKernel. - Marc

…

On Tue, May 5, 2026 at 7:24 PM Matthias Goerner ***@***.***> wrote: *unhyperbolic* left a comment (3-manifolds/SnapPy#134) <#134 (comment)> This still causes some test failures and I have some naming questions. The idea is that the user-facing classes Triangulation[HP] and Manifold[HP] subclass from 1. a class defined in python that brings in all the python methods and 2. a cython class bringing in the cython wrapped methods. 1. Is called Triangulation/ManifoldMixIn. I am somewhat happy with the name, but am open to suggestions. I wonder whether something like Triangulation/ManifoldPyBase are better in that they reflect the intention that this is where you add the python methods shared between Triangulation[HP] and Manifold[HP]. 2. Is called Triangulation and Manifold in the cython module. I find it actually somewhat confusing that they have the same name as the user-facing classes. Using the fully qualified name such as extensions.SnapPy.Triangulation makes it a bit better. But I wonder whether they should just be called _Triangulation and _Manifold in cython or Triangulation/ManifoldCyBase to reflect the intention better. On a related note: tests in extensions.SnapPy test against the cython base class and not the user-facing class. I would prefer to test against the user-facing class. This affects things like Manifold.is_isometric_to which we expect to work when mixing Manifold and ManifoldHP. My first though was that after renaming, say, Triangulation in extensions.SnapPy, we replace extensions.SnapPy._triangulation_class = Triangulation by extensions.SnapPy.Triangulation = Triangulation. Then setting extensions.SnapPyHP.Triangulation = TriangulationHP would allow both the cython code to call Triangulation(...) to generate the respective python class and would work in the tests. But it feels wrong to me. So I think we still stick with extensions.SnapPy._triangulation_class but when testing extensions.SnapPy set the globals to use the python super classes Triangulation[HP] and Manifold[HP]. Let me know whether you agree with my thoughts and which of the above naming choices you prefer. — Reply to this email directly, view it on GitHub <#134 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ6CP5JQSW6ZJTJNGMGHIT4ZKA5XAVCNFSM6AAAAACYSJ7JHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGOBUGE4TGNBZGE> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because your review was requested.Message ID: ***@***.***>

[unhyperbolic]

My initial reaction is that it would be better to have python classes named TriangulationBase and ManifoldBase and Cython classes named ManifoldKernel and TriangulationKernel.

I like the proposal. KernelTriangulation somehow sounds better but is inconsistent with Base being a suffix...

[culler]

BaseManifold doesn't seem so bad. - Marc

…

On Tue, May 5, 2026, 10:47 PM Matthias Goerner ***@***.***> wrote: *unhyperbolic* left a comment (3-manifolds/SnapPy#134) <#134 (comment)> My initial reaction is that it would be better to have python classes named TriangulationBase and ManifoldBase and Cython classes named ManifoldKernel and TriangulationKernel. I like the proposal. KernelTriangulation somehow sounds better but is inconsistent with Base being a suffix... — Reply to this email directly, view it on GitHub <#134 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ6CP6A3BAWZ2XGK7KYCMT4ZKYWTAVCNFSM6AAAAACYSJ7JHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGOBUHEYTKOJQGU> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because your review was requested.Message ID: ***@***.***>

[NathanDunfield]

In general, I think "ManifoldBase" and "KernelManifold" and similar are better names than "ManifoldMixIn" or "_Manifold". Personally, I'm fine with using "ManifoldBase" and "KernelManifold" despite the asymmetry in where the modifier is. If we want to be consistent, I'd lean towards "BaseManifold/KernelManifold" over "ManifoldBase/ManifoldKernel".

[NathanDunfield]

On a related note: tests in extensions.SnapPy test against the cython base class and not the user-facing class. I would prefer to test against the user-facing class. This affects things like Manifold.is_isometric_to which we expect to work when mixing Manifold and ManifoldHP.

I agree it is better to test against the user-facing class. However, rather than what you propose, is it possible to do that by just running the doctests on the final Manifold and ManifoldHP classes rather than the Cython base classes?

[unhyperbolic]

I went ahead and checked-in the new python base classes.

Having gone through the exercise of renaming extensions.SnapPy.Triangulation/Manifold to KernelTriangulation/Manifold, I am a bit hesitant about this rename:

1. extensions.SnapPy.Triangulation/Manifold is more than just purely wrapping the kernel. It adds a lot of its own logic and uses different names (e.g., canonize instead of proto_canonize, symmetry_group instead of compute_symmetry_group).
2. I find some of the results such as "cdef c_Triangulation t\ncdef KernelTriangulation tri" confusing.

[NathanDunfield]

Hmm, perhaps CythonManifold/CythonTriangulation is a better choice then?

[unhyperbolic]

Hmm, perhaps CythonManifold/CythonTriangulation is a better choice then?

My first though was along those lines: Triangulation/ManifoldCyBase. But CythonTriangulation/Manifold has a nicer ring to it. CCuspNeighborhood -> CyCuspNeighborhood along the same lines then, I guess.

[culler]

I don't see why it is a problem that a KernelManifold does more that just wrap kernel methods.

What is confusing about the two cdef lines? The use of the c_ prefix was to indicate that t is a C struct, which it is.