Opened 4 months ago

Closed 4 months ago

#30289 closed defect (fixed)

Error in display of a continuous map between open intervals

Reported by: tscrim Owned by:
Priority: major Milestone: sage-9.2
Component: manifolds Keywords: equation display
Cc: egourgoulhon, gh-mjungmath, slelievre Merged in:
Authors: Michael Jung Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: be7a363 (Commits) Commit: be7a363cd90d8802945094d8084242101e711538
Dependencies: Stopgaps:

Description (last modified by slelievre)

Define a continuous map between two intervals:

sage: R.<t> = RealLine()
sage: I = R.open_interval(0, 2*pi)
sage: J = R.open_interval(2*pi, 6*pi)
sage: h = J.continuous_map(I, ((t-2*pi)/2,), name='h')

Before this ticket, it displays incorrectly:

sage: h.display()
h: (2*pi, 6*pi) --> (0, 2*pi)
   t |-->

After this ticket, it displays correctly:

sage: h.display()
h: (2*pi, 6*pi) --> (0, 2*pi)
   t |--> t = -pi + 1/2*t

Change History (16)

comment:1 Changed 4 months ago by tscrim

  • Description modified (diff)

comment:2 Changed 4 months ago by gh-mjungmath

Removing the error block in starting in line 1069 in continuous_map.py then yields the following error:

sage: R.<t> = RealLine()
sage: I = R.open_interval(0, 2*pi)
sage: J = R.open_interval(2*pi, 6*pi)
sage: h = J.continuous_map(I, ((t-2*pi)/2,), name='h')
sage: h.display()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-67d00ebd9395> in <module>()
      3 J = R.open_interval(Integer(2)*pi, Integer(6)*pi)
      4 h = J.continuous_map(I, ((t-Integer(2)*pi)/Integer(2),), name='h')
----> 5 h.display()

/home/michi/Projekte/sage-devel/local/lib/python3.7/site-packages/sage/manifolds/continuous_map.py in display(self, chart1, chart2)
   1113                 for ch1 in self._domain._top_charts:
   1114                     for ch2 in self._codomain.atlas():
-> 1115                         _display_expression(self, ch1, ch2, result)
   1116             else:
   1117                 for ch1 in self._domain._top_charts:

/home/michi/Projekte/sage-devel/local/lib/python3.7/site-packages/sage/manifolds/continuous_map.py in _display_expression(self, chart1, chart2, result)
   1086                 if len(expression) == 1:
   1087                     result._txt += repr(coords2[0]) + " = " + \
-> 1088                                   repr(expression[0]) + "\n"
   1089                     result._latex += latex(coords2[0]) + " = " + \
   1090                                     latex(coord_func[0]) + r"\\"

TypeError: 'sage.symbolic.expression.Expression' object is not subscriptable

Imho, this try-except block should be removed and replaced by another solution (or at least restricted to a smaller snippet of code) since it can mask bugs like these.

Furthermore, I just noticed that picewise defined continuous maps (not quite the unusal case) could perhaps not be displayed properly (cf. #28554 for scalar fields).

Last edited 4 months ago by gh-mjungmath (previous) (diff)

comment:3 Changed 4 months ago by gh-mjungmath

Nevertheless, the error message is somewhat strange:

sage: R.<t> = RealLine()
sage: I = R.open_interval(0, 2*pi)
sage: J = R.open_interval(2*pi, 6*pi)
sage: h = J.continuous_map(I, ((t-2*pi)/2,), name='h')
sage: c_I = I.atlas()[0]
sage: c_J = J.atlas()[0]
sage: coord_func = h.coord_functions(c_J, c_I)
sage: expression = coord_func.expr()
sage: type(expression)
<class 'tuple'>

The variable expression should be a tuple and hence be subscriptable.

Version 2, edited 4 months ago by gh-mjungmath (previous) (next) (diff)

comment:4 Changed 4 months ago by gh-mjungmath

  • Branch set to u/gh-mjungmath/error_in_display_of_a_continuous_map_between_open_intervals

comment:5 Changed 4 months ago by gh-mjungmath

  • Authors set to Michael Jung
  • Commit set to 2d3664b5e40b7b34e4af185acc9ab09e1e265978
  • Status changed from new to needs_review

New commits:

2d3664bTrac #30289: fix display for one variable

comment:6 Changed 4 months ago by tscrim

Shouldn't the same change also be done for the result._latex?

Also, please add a doctest.

comment:7 Changed 4 months ago by gh-mjungmath

Oh yes, sure. I was too impatient and haven't double checked. Sorry.

comment:8 Changed 4 months ago by gh-mjungmath

  • Status changed from needs_review to needs_work

comment:9 Changed 4 months ago by git

  • Commit changed from 2d3664b5e40b7b34e4af185acc9ab09e1e265978 to 60040b57a4696ba5ed42e2e71457e1b020a8de1e

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

60040b5Trac #30289: doctest + latex output

comment:10 Changed 4 months ago by gh-mjungmath

  • Status changed from needs_work to needs_review

comment:11 Changed 4 months ago by git

  • Commit changed from 60040b57a4696ba5ed42e2e71457e1b020a8de1e to be7a363cd90d8802945094d8084242101e711538

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

be7a363Trac #30289: try-except shifted + code improvements

comment:12 Changed 4 months ago by gh-mjungmath

This should be even better. I have shifted the try-except block. When an error is raised after this block, this must be a bug and therefore exposed as such.

Last edited 4 months ago by gh-mjungmath (previous) (diff)

comment:13 Changed 4 months ago by tscrim

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

That is the danger of putting too much stuff into a try-except block. Thank you for the quick fix. LGTM.

comment:14 Changed 4 months ago by slelievre

  • Cc slelievre added
  • Description modified (diff)

comment:15 Changed 4 months ago by slelievre

  • Description modified (diff)

comment:16 Changed 4 months ago by vbraun

  • Branch changed from u/gh-mjungmath/error_in_display_of_a_continuous_map_between_open_intervals to be7a363cd90d8802945094d8084242101e711538
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.