Opened 2 years ago
Closed 2 years ago
#30289 closed defect (fixed)
Error in display of a continuous map between open intervals
Reported by:  tscrim  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  manifolds  Keywords:  equation display 
Cc:  egourgoulhon, ghmjungmath, slelievre  Merged in:  
Authors:  Michael Jung  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  be7a363 (Commits, GitHub, GitLab)  Commit:  be7a363cd90d8802945094d8084242101e711538 
Dependencies:  Stopgaps: 
Description (last modified by )
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, ((t2*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 2 years ago by
 Description modified (diff)
comment:2 Changed 2 years ago by
comment:3 Changed 2 years ago by
Ah okay, it's the variable coords2
. The bug can be fixed very quickly.
My other points still stand for discussion. :)
comment:4 Changed 2 years ago by
 Branch set to u/ghmjungmath/error_in_display_of_a_continuous_map_between_open_intervals
comment:5 Changed 2 years ago by
 Commit set to 2d3664b5e40b7b34e4af185acc9ab09e1e265978
 Status changed from new to needs_review
New commits:
2d3664b  Trac #30289: fix display for one variable

comment:6 Changed 2 years ago by
Shouldn't the same change also be done for the result._latex
?
Also, please add a doctest.
comment:7 Changed 2 years ago by
Oh yes, sure. I was too impatient and haven't double checked. Sorry.
comment:8 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:9 Changed 2 years ago by
 Commit changed from 2d3664b5e40b7b34e4af185acc9ab09e1e265978 to 60040b57a4696ba5ed42e2e71457e1b020a8de1e
Branch pushed to git repo; I updated commit sha1. New commits:
60040b5  Trac #30289: doctest + latex output

comment:10 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:11 Changed 2 years ago by
 Commit changed from 60040b57a4696ba5ed42e2e71457e1b020a8de1e to be7a363cd90d8802945094d8084242101e711538
Branch pushed to git repo; I updated commit sha1. New commits:
be7a363  Trac #30289: tryexcept shifted + code improvements

comment:12 Changed 2 years ago by
This should be even better. I have shifted the tryexcept block. When an error is raised after this block, this must be a bug and therefore exposed as such.
comment:13 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
That is the danger of putting too much stuff into a tryexcept block. Thank you for the quick fix. LGTM.
comment:14 Changed 2 years ago by
 Cc slelievre added
 Description modified (diff)
comment:15 Changed 2 years ago by
 Description modified (diff)
comment:16 Changed 2 years ago by
 Branch changed from u/ghmjungmath/error_in_display_of_a_continuous_map_between_open_intervals to be7a363cd90d8802945094d8084242101e711538
 Resolution set to fixed
 Status changed from positive_review to closed
Removing the error block in starting in line 1069 in
continuous_map.py
then yields the following error:Imho, this tryexcept 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).