Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update nbconvert pinning #1161

Merged
merged 9 commits into from
Oct 4, 2022

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Jun 16, 2022

Update nbconvert pinning

References

Resolve #1160

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch martinRenou/voila/test_nbconvert_master

@martinRenou martinRenou marked this pull request as draft June 27, 2022 12:56
@jtpio
Copy link
Member

jtpio commented Sep 29, 2022

Maybe this PR could now be about raising the upper bounds for nbclient and nbconvert?

voila/setup.cfg

Lines 39 to 40 in 55d0cb0

nbclient>=0.4.0,<0.6
nbconvert>=6.4.5,<7

@jtpio jtpio added this to the 0.4.0 milestone Sep 29, 2022
@martinRenou martinRenou changed the title UI-tests against nbconvert master Update nbconvert and nbclient pinning Sep 30, 2022
@martinRenou martinRenou marked this pull request as ready for review September 30, 2022 14:44
@martinRenou martinRenou added the enhancement New feature or request label Sep 30, 2022
@martinRenou martinRenou changed the title Update nbconvert and nbclient pinning Update nbconvert pinning Sep 30, 2022
@martinRenou
Copy link
Member Author

martinRenou commented Sep 30, 2022

I can't understand yet why the tests fail in the CI with this AttributeError: module 'mistune' has no attribute 'Renderer'.

I'm also running Mistune 2.0.4 locally and it works just fine to run the tests.

@jtpio
Copy link
Member

jtpio commented Sep 30, 2022

I just tried in a clean environment (Gitpod) and also getting the same error as in the CI tests:

image

image

@jtpio
Copy link
Member

jtpio commented Sep 30, 2022

Looking at the diff in jupyter/nbconvert#1764, we might need to update this line to something like mistune.HTMLRenderer:

markdown_renderer_class = traitlets.Type('mistune.Renderer').tag(config=True)

@martinRenou
Copy link
Member Author

I was in this weird installation where conda list would show me Mistune 2.0.4 while I was actually using 0.8.

@martinRenou martinRenou marked this pull request as draft October 3, 2022 08:32
@martinRenou martinRenou added the dependencies Pull requests that update a dependency file label Oct 3, 2022
@martinRenou
Copy link
Member Author

The failing test seems to be due to the change in validate/normalize logic introduced by jupyter/nbconvert@8c1146d and jupyter/nbformat#282.

@martinRenou
Copy link
Member Author

The failing test seems to be due to the change in validate/normalize logic introduced by jupyter/nbconvert@8c1146d and jupyter/nbformat#282.

Indeed the failing test was originated from those, because nbconvert is now less permissive with invalid Notebooks, and papermill generates an invalid Notebook (see nteract/papermill#686 (comment)). Bumping our test Notebook version fixes it.

@martinRenou martinRenou marked this pull request as ready for review October 4, 2022 07:25
voila/exporter.py Outdated Show resolved Hide resolved
We need a class that can accept a contents_manager as parameter, this is
not the case anymore with the new Mistune HTMLRenderer so we need to be
more specific with the required trait
pyproject.toml Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

Results table
Test file voila-tree-classic.ipynb voila-tree-light.ipynb voila-tree-dark.ipynb voila-tree-miami.ipynb basics.ipynb bqplot.ipynb dashboard.ipynb gridspecLayout.ipynb interactive.ipynb ipympl.ipynb ipyvolume.ipynb multiple_widgets.ipynb query-strings.ipynb reveal.ipynb
Render
chromium
actual 110 <- [124 - 152 - 191] -> 327 65 <- [69 - 76 - 90] -> 114 62 <- [67 - 76 - 89] -> 120 60 <- [64 - 71 - 82] -> 108 2337 <- [2405 - 2450 - 2603] -> 3190 2090 <- [2119 - 2121 - 2216] -> 2441 2335 <- [2394 - 2454 - 2465] -> 2704 2338 <- [2399 - 2463 - 2538] -> 2644 1786 <- [1796 - 1811 - 1860] -> 2071 2998 <- [3076 - 3257 - 3533] -> 4651 7927 <- [7934 - 7938 - 7960] -> 8020 5758 <- [5793 - 5808 - 5913] -> 6160 1105 <- [1233 - 1249 - 1264] -> 1266 2103 <- [2152 - 2191 - 2199] -> 2205
expected 3379 <- [3442 - 3517 - 3701] -> 3876 2976 <- [3227 - 3321 - 3421] -> 3604 3608 <- [3623 - 3709 - 3793] -> 3825 4453 <- [4453 - 4523 - 4661] -> 4748 2559 <- [2655 - 2656 - 2660] -> 2674 3982 <- [4079 - 4213 - 4356] -> 4743 12183 <- [18509 - 19553 - 20811] -> 21515 15319 <- [15660 - 15796 - 15912] -> 16056 1517 <- [1920 - 1997 - 2103] -> 2113

❗ Test metadata have changed
--- /dev/fd/63	2022-10-04 08:01:49.093725752 +0000
+++ /dev/fd/62	2022-10-04 08:01:49.093725752 +0000
@@ -4,37 +4,37 @@
     "BENCHMARK_REFERENCE": "actual"
   },
   "browsers": {
-    "chromium": "97.0.4666.0"
+    "chromium": "94.0.4595.0"
   },
   "systemInformation": {
     "cpu": {
-      "brand": "Xeon® Platinum 8370C",
+      "brand": "Xeon® E5-2673 v3",
       "cache": {
-        "l1d": 98304,
+        "l1d": 65536,
         "l1i": 65536,
-        "l2": 2097152,
-        "l3": 50331648
+        "l2": 524288,
+        "l3": 31457280
       },
       "cores": 2,
       "family": "6",
-      "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single pti fsgsbase bmi1 hle avx2 smep bmi2 erms invpcid rtm avx512f avx512dq rdseed adx smap clflushopt avx512cd avx512bw avx512vl xsaveopt xsavec xsaves md_clear",
+      "flags": "fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm invpcid_single pti fsgsbase bmi1 avx2 smep bmi2 erms invpcid xsaveopt md_clear",
       "governor": "",
       "manufacturer": "Intel®",
-      "model": "106",
+      "model": "63",
       "physicalCores": 2,
       "processors": 1,
       "revision": "",
       "socket": "",
-      "speed": 2.8,
+      "speed": 2.4,
       "speedMax": null,
       "speedMin": null,
-      "stepping": "6",
+      "stepping": "2",
       "vendor": "GenuineIntel",
       "virtualization": false,
       "voltage": ""
     },
     "mem": {
-      "total": 7281315840
+      "total": 7291699200
     },
     "osInfo": {
       "arch": "x64",
@@ -42,11 +42,11 @@
       "codename": "Focal Fossa",
       "codepage": "UTF-8",
       "distro": "Ubuntu",
-      "kernel": "5.15.0-1020-azure",
+      "kernel": "5.8.0-1040-azure",
       "logofile": "ubuntu",
       "platform": "linux",
-      "release": "20.04.5 LTS",
-      "serial": "1d24188fee7f4944b0cb9c8863cdca51",
+      "release": "20.04.3 LTS",
+      "serial": "cfc067bfcb844f35865e279a1b0e66c5",
       "servicepack": "",
       "uefi": false
     }

@martinRenou martinRenou requested a review from jtpio October 4, 2022 08:15
Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jtpio jtpio merged commit 8244587 into voila-dashboards:main Oct 4, 2022
@jtpio
Copy link
Member

jtpio commented Oct 4, 2022

Starting a new pre-release now: #1225

@martinRenou martinRenou deleted the test_nbconvert_master branch October 4, 2022 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test against nbconvert master
3 participants