Opened 2 years ago
Last modified 2 years ago
#30289 closed defect
Error in display of a continuous map between open intervals — at Version 15
Reported by:  Travis Scrimshaw  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  manifolds  Keywords:  equation display 
Cc:  Eric Gourgoulhon, Michael Jung, Samuel Lelièvre  Merged in:  
Authors:  Michael Jung  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  u/ghmjungmath/error_in_display_of_a_continuous_map_between_open_intervals (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 (15)
comment:1 Changed 2 years ago by
Description:  modified (diff) 

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:  → u/ghmjungmath/error_in_display_of_a_continuous_map_between_open_intervals 

comment:5 Changed 2 years ago by
Authors:  → Michael Jung 

Commit:  → 2d3664b5e40b7b34e4af185acc9ab09e1e265978 
Status:  new → 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:  needs_review → needs_work 

comment:9 Changed 2 years ago by
Commit:  2d3664b5e40b7b34e4af185acc9ab09e1e265978 → 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:  needs_work → needs_review 

comment:11 Changed 2 years ago by
Commit:  60040b57a4696ba5ed42e2e71457e1b020a8de1e → 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:  → Travis Scrimshaw 

Status:  needs_review → 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:  Samuel Lelièvre added 

Description:  modified (diff) 
comment:15 Changed 2 years ago by
Description:  modified (diff) 

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).