Immutability of chart functions

[mjungmath]

Immutability of chart functions, see #30261.

Depends on #31181 Depends on #31182

CC: @egourgoulhon @tscrim @mkoeppe

Component: manifolds

Keywords: immutability

Author: Michael Jung

Branch/Commit: e855ebc

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/30310

[mjungmath]

Branch: u/gh-mjungmath/immutability_of_chart_functions

[mjungmath]

Description changed:

--- 
+++ 
@@ -1 +1 @@
-
+Immutability of chart functions, see #30261.

[mjungmath]

Author: Michael Jung

[mjungmath]

New commits:

286d3c6	FreeModuleTensor, Vector: Subclass a new class ModuleElementWithMutability
9df3d22	sage.tensor.modules: Make all zero() and one() elements immutable
4373ea2	FreeModuleTensor: Replace special casing of the immutable zero by is_immutable
a2ee8be	ModuleElementWithMutability.is_immutable, is_mutable: Change def to cpdef
8600c2d	Merge branch 't/30181/immutable_elements_of_freemoduletensor' into t/30310/immutability_of_chart_functions
76229bd	Trac #30310: immutability of chart functions

[mjungmath]

Commit: 76229bd

[mjungmath]

Changed keywords from none to immutability

[mjungmath]

Dependencies: #30181

[egourgoulhon]

comment:4

Why is #30181 a dependency of this ticket? A priori chart functions are totally independent from tensor fields. They even exist on pure topological manifolds.

[mjungmath]

comment:5

It is because of

-from sage.structure.element import AlgebraElement
+from sage.structure.element import AlgebraElement, ModuleElementWithMutability
...
-class ChartFunction(AlgebraElement):
+class ChartFunction(AlgebraElement, ModuleElementWithMutability):

[mjungmath]

comment:6

ModuleElementWithMutability is first introduced in #30181.

[tscrim]

comment:7

I feel like the MultiCoordFunction should inherit from Mutability rather than copy the same code.

[mjungmath]

comment:8

It'd be the same issue like we had for affine connections (#30280): __reduce__ (#30281).

[tscrim]

comment:9

I know, but as a stop-gap, you could instead implement a __reduce__. I don't really like this duplication. Hopefully this week I can get around to properly doing #30281...

[egourgoulhon]

comment:10

Replying to @mjungmath:

It is because of

-from sage.structure.element import AlgebraElement
+from sage.structure.element import AlgebraElement, ModuleElementWithMutability
...
-class ChartFunction(AlgebraElement):
+class ChartFunction(AlgebraElement, ModuleElementWithMutability):

Ah yes, thanks.

[mjungmath]

comment:12

I tried to remove the redundant code and inherit from Mutability after #30281. For some reason, I still get a pickling error during doctest.

Furthermore, the code seems broken: is_immutable does not return anything:

    def is_immutable(self):
        """
        ...
        """
        self._is_immutable

When did that happen?

[mjungmath]

comment:14

I opened a ticket for the missing return: #31181.

Furthermore, I've opened a ticket for the still remaining pickling test error: #31182.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 76229bd to 09086cf

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

09086cf

Trac #30310: inherit from Mutability

[mjungmath]

Changed dependencies from #30181 to #31181, #31182

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 09086cf to 6c76b66

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

6c76b66

Trac #30310: AssertionError -> ValueError

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

7247620	Merge branch 'develop' into t/30310/immutability_of_chart_functions
5ecbf3a	Trac #31181: return added
8b20fc4	Merge branch 't/31181/mutability_class_does_not_return_is_immutable' into t/30310/immutability_of_chart_functions
4c33935	Trac #31196: minor code improvements, py3 compatibility, documentation improved
e5228d3	Trac #31196: cpdef require methods + example added
d957f73	Trac #31196: unnecessary line in docstring removed
d6d6ba4	Trac #31182: `__getstate__` and __setstate__
6cbd1fd	Trac #31182: doctests added for `__setstate__` and __getstate__
848e96b	Merge branch 't/31182/mutability_class_and_pickling' into t/30310/immutability_of_chart_functions

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 6c76b66 to 848e96b

[mjungmath]

comment:19

Green patchbot.

[tscrim]

comment:20

I think you are better off explicitly calling

ModuleElementWithMutability.__init__(self, parent)

[mjungmath]

comment:21

Yes thanks, this should be better.

[mjungmath]

comment:22

Pushed.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 848e96b to e855ebc

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

e855ebc

Trac #30310: init ModuleElementWithMutability directly

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:24

Green bot => positive review.

[mjungmath]

comment:26

Thank you.

[vbraun]

Changed branch from u/gh-mjungmath/immutability_of_chart_functions to e855ebc