Opened 5 years ago

Closed 4 years ago

#22668 closed defect (fixed)

HTML widget without description

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.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, GitHub, GitLab) Commit: c8018213736cb9f12794d49b5296e0b980e18aaf
Dependencies: #22644 Stopgaps:

Status badges

Description (last modified by jdemeyer)

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 5 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/html_widget_without_description

comment:3 Changed 5 years ago by git

  • Commit set to 69b2397f5f436797e5db3b866f65f29fb08ea9dd

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

69b2397HTML widget without description

comment:4 Changed 5 years ago by jdemeyer

  • Status changed from new to needs_review

comment:5 follow-up: Changed 4 years ago by slabbe

  • 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 slabbe

  • Keywords days86 added

comment:7 in reply to: ↑ 5 Changed 4 years ago by jdemeyer

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 slabbe

  • 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

I added two doctests on a new public branch. If you agree with it, then you may set this ticket to positive review.


New commits:

8c5f0aaMerged 22668 into 8.0.beta2
41a68f122668: adding two doctests

comment:9 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:10 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

on OSX:

sage -t --long --warn-long 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 --warn-long 126.8 src/sage/misc/rest_index_of_methods.py

comment:11 Changed 4 years ago by jdemeyer

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 jdemeyer

  • Dependencies set to #22644

comment:13 Changed 4 years ago by git

  • Commit changed from 41a68f16787687b5446b05ed81a30f178d1ae6ea to c8018213736cb9f12794d49b5296e0b980e18aaf

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cc924cfMerge commit '0d0b1492867859af17df1418c2331be287025bf8' into t/22644/fix_and_test_interact_library
9b4d47eFix and test interact library
7168721Merge commit 'b72155dc1cf0c0eb063dafc578a022beff2d9a14' into t/22644/fix_and_test_interact_library
9cfe2aaMerge tag '8.0.beta3' into t/22644/fix_and_test_interact_library
9658bb9HTML widget without description
dcdba7822668: adding two doctests
c801821Fix doctests in interact library

comment:14 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:15 Changed 4 years ago by slabbe

  • Status changed from needs_review to positive_review

comment:16 Changed 4 years ago by vbraun

  • Branch changed from public/22668 to c8018213736cb9f12794d49b5296e0b980e18aaf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.