#32009 closed enhancement (fixed)

Eliminate direct use of the Chart._domain attribute

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: Michael Jung, Eric Gourgoulhon, Volker Braun Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw, Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: u/mkoeppe/eliminate_direct_use_of_the_chart__domain_attribute (Commits, GitHub, GitLab) Commit: bf62543e71954a3ad3acecd4a3089692a4eea9f5
Dependencies: #32112, #30473 Stopgaps:

Status badges

Description

... using the method .domain() instead

This is preparation for #31894 - when _domain is a Cython attribute, it will not be directly accessible from Python.

Change History (14)

comment:1 Changed 18 months ago by Matthias Köppe

Branch: u/mkoeppe/eliminate_direct_use_of_the_chart__domain_attribute

comment:2 Changed 18 months ago by Matthias Köppe

Commit: 8ba174cc014d892ab8defbb9453e847067af5b13
Status: newneeds_review

New commits:

8ba174cEliminate direct use of Chart._domain

comment:3 Changed 18 months ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM.

comment:4 Changed 18 months ago by Matthias Köppe

Thanks!

comment:5 Changed 17 months ago by Volker Braun

Status: positive_reviewneeds_work
sage -t --long --warn-long 43.2 --random-seed=0 src/sage/manifolds/chart.py
**********************************************************************
File "src/sage/manifolds/chart.py", line 184, in sage.manifolds.chart.Chart
Failed example:
    XN.<Z1,Z2> = N.chart('Z1:period=1+2*I Z2')
Exception raised:
    Traceback (most recent call last):
      File "sage/misc/cachefunc.pyx", line 996, in sage.misc.cachefunc.CachedFunction.__call__ (build/cythonized/sage/misc/cachefunc.c:5974)
        return self.cache[k]
      File "sage/misc/weak_dict.pyx", line 704, in sage.misc.weak_dict.WeakValueDictionary.__getitem__ (build/cythonized/sage/misc/weak_dict.c:3736)
        raise KeyError(k)
    KeyError: ((<class 'sage.manifolds.chart.Chart'>, Complex 2-dimensional topological manifold N), (('calc_method', 'SR'), ('coordinates', 'Z1:period=1+2*I Z2'), ('names', ('Z1', 'Z2'))))

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/doctest/forker.py", line 718, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/doctest/forker.py", line 1137, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.manifolds.chart.Chart[22]>", line 1, in <module>
        XN = N.chart('Z1:period=1+2*I Z2', names=('Z1', 'Z2',)); (Z1, Z2,) = XN._first_ngens(2)
      File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/manifolds/manifold.py", line 1571, in chart
        return self._structure.chart(self, coordinates=coordinates,
      File "sage/misc/classcall_metaclass.pyx", line 322, in sage.misc.classcall_metaclass.ClasscallMetaclass.__call__ (build/cythonized/sage/misc/classcall_metaclass.c:1761)
        return cls.classcall(cls, *args, **kwds)
      File "sage/misc/cachefunc.pyx", line 1001, in sage.misc.cachefunc.CachedFunction.__call__ (build/cythonized/sage/misc/cachefunc.c:6100)
        w = self.f(*args, **kwds)
      File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/structure/unique_representation.py", line 1007, in __classcall__
        instance = typecall(cls, *args, **options)
      File "sage/misc/classcall_metaclass.pyx", line 486, in sage.misc.classcall_metaclass.typecall (build/cythonized/sage/misc/classcall_metaclass.c:2223)
        return (<PyTypeObject*>type).tp_call(cls, args, kwds)
      File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/manifolds/chart.py", line 309, in __init__
        self._init_coordinates(coord_list)
      File "/home/release/Sage/local/lib64/python3.9/site-packages/sage/manifolds/chart.py", line 389, in _init_coordinates
        if domain.base_field_type() in ['real', 'complex']:
    NameError: name 'domain' is not defined
**********************************************************************
[...]

comment:6 Changed 17 months ago by Matthias Köppe

Dependencies: #32112

comment:7 Changed 17 months ago by git

Commit: 8ba174cc014d892ab8defbb9453e847067af5b13907c9bc9a2cdf35588a712e14ace6e7ab41274da

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

f43b358Chart._init_coordinates: Fix up use of domain
4e316e9Fix bug in Chart.__init__ regarding the determination of top charts (Trac #32112)
907c9bcMerge #32112

comment:8 Changed 17 months ago by Matthias Köppe

Status: needs_workneeds_review

comment:9 Changed 17 months ago by Eric Gourgoulhon

Reviewers: Travis ScrimshawTravis Scrimshaw, Eric Gourgoulhon
Status: needs_reviewpositive_review

LGTM.

comment:10 Changed 17 months ago by Matthias Köppe

Thanks!

comment:11 Changed 17 months ago by Matthias Köppe

Dependencies: #32112#32112, #30473

comment:12 Changed 17 months ago by git

Commit: 907c9bc9a2cdf35588a712e14ace6e7ab41274dabf62543e71954a3ad3acecd4a3089692a4eea9f5
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

82f12e2Merge #32013
0f60232ConditionSet: Do not sort the conditions, just use _stable_uniq
69d045aConditionSet: In doctests, do not rename ZZ^2 etc.
daeb91esrc/sage/sets/set.py: Fix docstring markup
2cf2199Merge #32015
1eb270asrc/sage/docs/conf.py: Add more \ensuremath to \DeclareUnicodeCharacter
2682469src/sage/interfaces/sympy_wrapper.py: Use Family, not Set, in doctests to make sure that the SageSet wrapper is actually used
753babbSet_object_enumerated._sympy_: Translate empty sets to EmptySet
141ecdeMerge #32015
bf62543Merge branch 't/32089/conditionset__conditionset_callable_symbolic_expression' into t/32009/eliminate_direct_use_of_the_chart__domain_attribute

comment:13 Changed 17 months ago by Matthias Köppe

Status: needs_reviewpositive_review

Merged #30473 to resolve merge conflict.

comment:14 Changed 16 months ago by Matthias Köppe

Cc: Volker Braun added
Resolution: fixed
Status: positive_reviewclosed

Apparently this has been merged as part of #32089.

Note: See TracTickets for help on using tickets.