Opened 4 years ago
Closed 4 years ago
#22668 closed defect (fixed)
HTML widget without description
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  notebook  Keywords:  days86 
Cc:  kcrisman  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Sébastien Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  c801821 (Commits)  Commit:  c8018213736cb9f12794d49b5296e0b980e18aaf 
Dependencies:  #22644  Stopgaps: 
Description (last modified by )
In SageNB speak: text_control
without label.
The SageNB text_control
is aliased to the HTML
widget in Jupyter. This makes sense, except that the HTML
has a description
which becomes a label. This does not look right when used in interacts: it is common in SageNB to do things like
@interact def foo(txt=text_control("<h1>Interact for foo</h1>"))
where the "Interact for foo" is displayed as a heading for the interact. You don't want "txt" to appear as label for this heading.
Change History (16)
comment:1 Changed 4 years ago by
 Description modified (diff)
comment:2 Changed 4 years ago by
 Branch set to u/jdemeyer/html_widget_without_description
comment:3 Changed 4 years ago by
 Commit set to 69b2397f5f436797e5db3b866f65f29fb08ea9dd
comment:4 Changed 4 years ago by
 Status changed from new to needs_review
comment:5 followup: ↓ 7 Changed 4 years ago by
 Status changed from needs_review to needs_work
I was able to reproduce the problem in Jupyter ("txt" tag appears in the output). This ticket fixes this issue. I also made sure that there is no regression on the behavior of text_control
inside the old sagenb notebook. To me this is a perfect.
Before setting this to positive review, I just want to make sure about the small regression in coverage introduced by this ticket:
Decreased doctests in repl/ipython_kernel/widgets.py: from 7 / 7 = 100% to 7 / 9 = 77%
due to the new methods:
+ @property
+ def description(self):
+ return ''
+
+ @description.setter
+ def description(self, value):
+ pass
I know that they are tested few lines above, but I prefer not see the coverage script tell us thousands of time that there is a coverage problem in this file. Do you accept to fix this?
comment:6 Changed 4 years ago by
 Keywords days86 added
comment:7 in reply to: ↑ 5 Changed 4 years ago by
And which doctest would you like me to add? I think no doctest is still better than a pointless artificial test just to satisfy the coverage script.
comment:8 Changed 4 years ago by
 Branch changed from u/jdemeyer/html_widget_without_description to public/22668
 Commit changed from 69b2397f5f436797e5db3b866f65f29fb08ea9dd to 41a68f16787687b5446b05ed81a30f178d1ae6ea
 Reviewers set to Sébastien Labbé
 Status changed from needs_work to needs_review
comment:9 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:10 Changed 4 years ago by
 Status changed from positive_review to needs_work
on OSX:
sage t long warnlong 126.8 src/sage/interacts/test_jupyter.rst ********************************************************************** File "src/sage/interacts/test_jupyter.rst", line 11, in sage.interacts.test_jupyter Failed example: interacts.geometry.special_points() Expected: Interactive function <function special_points at ...> with 10 widgets title: HTML(value=u'<h2>Special points in triangle</h2>', description=u'title') a0: IntSlider(value=30, min=0, max=360, step=1, description=u'A') a1: IntSlider(value=180, min=0, max=360, step=1, description=u'B') a2: IntSlider(value=300, min=0, max=360, step=1, description=u'C') show_median: Checkbox(value=False, description=u'Medians') show_pb: Checkbox(value=False, description=u'Perpendicular Bisectors') show_alt: Checkbox(value=False, description=u'Altitudes') show_ab: Checkbox(value=False, description=u'Angle Bisectors') show_incircle: Checkbox(value=False, description=u'Incircle') show_euler: Checkbox(value=False, description=u"Euler's Line") Got: Interactive function <function special_points at 0x22ca80320> with 10 widgets title: HTMLText(value=u'<h2>Special points in triangle</h2>') a0: IntSlider(value=30, min=0, max=360, step=1, description=u'A') a1: IntSlider(value=180, min=0, max=360, step=1, description=u'B') a2: IntSlider(value=300, min=0, max=360, step=1, description=u'C') show_median: Checkbox(value=False, description=u'Medians') show_pb: Checkbox(value=False, description=u'Perpendicular Bisectors') show_alt: Checkbox(value=False, description=u'Altitudes') show_ab: Checkbox(value=False, description=u'Angle Bisectors') show_incircle: Checkbox(value=False, description=u'Incircle') show_euler: Checkbox(value=False, description=u"Euler's Line") ********************************************************************** 1 item had failures: 1 of 6 in sage.interacts.test_jupyter [4 tests, 1 failure, 0.89 s] sage t long warnlong 126.8 src/sage/misc/rest_index_of_methods.py
comment:11 Changed 4 years ago by
This is a "conflict" with #22644 I guess (where this test was added). I will wait for the next beta.
comment:12 Changed 4 years ago by
 Dependencies set to #22644
comment:13 Changed 4 years ago by
 Commit changed from 41a68f16787687b5446b05ed81a30f178d1ae6ea to c8018213736cb9f12794d49b5296e0b980e18aaf
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cc924cf  Merge commit '0d0b1492867859af17df1418c2331be287025bf8' into t/22644/fix_and_test_interact_library

9b4d47e  Fix and test interact library

7168721  Merge commit 'b72155dc1cf0c0eb063dafc578a022beff2d9a14' into t/22644/fix_and_test_interact_library

9cfe2aa  Merge tag '8.0.beta3' into t/22644/fix_and_test_interact_library

9658bb9  HTML widget without description

dcdba78  22668: adding two doctests

c801821  Fix doctests in interact library

comment:14 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:15 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:16 Changed 4 years ago by
 Branch changed from public/22668 to c8018213736cb9f12794d49b5296e0b980e18aaf
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
HTML widget without description