Opened 4 months ago

Closed 3 months ago

#32009 closed enhancement (fixed)

Eliminate direct use of the Chart._domain attribute

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: gh-mjungmath, egourgoulhon, vbraun 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 4 months ago by mkoeppe

  • Branch set to u/mkoeppe/eliminate_direct_use_of_the_chart__domain_attribute

comment:2 Changed 4 months ago by mkoeppe

  • Commit set to 8ba174cc014d892ab8defbb9453e847067af5b13
  • Status changed from new to needs_review

New commits:

8ba174cEliminate direct use of Chart._domain

comment:3 Changed 4 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:4 Changed 4 months ago by mkoeppe

Thanks!

comment:5 Changed 4 months ago by vbraun

  • Status changed from positive_review to needs_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 4 months ago by mkoeppe

  • Dependencies set to #32112

comment:7 Changed 4 months ago by git

  • Commit changed from 8ba174cc014d892ab8defbb9453e847067af5b13 to 907c9bc9a2cdf35588a712e14ace6e7ab41274da

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 4 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:9 Changed 3 months ago by egourgoulhon

  • Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Eric Gourgoulhon
  • Status changed from needs_review to positive_review

LGTM.

comment:10 Changed 3 months ago by mkoeppe

Thanks!

comment:11 Changed 3 months ago by mkoeppe

  • Dependencies changed from #32112 to #32112, #30473

comment:12 Changed 3 months ago by git

  • Commit changed from 907c9bc9a2cdf35588a712e14ace6e7ab41274da to bf62543e71954a3ad3acecd4a3089692a4eea9f5
  • Status changed from positive_review to needs_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 3 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Merged #30473 to resolve merge conflict.

comment:14 Changed 3 months ago by mkoeppe

  • Cc vbraun added
  • Resolution set to fixed
  • Status changed from positive_review to closed

Apparently this has been merged as part of #32089.

Note: See TracTickets for help on using tickets.