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

Migrate to hatch #1197

Merged
merged 18 commits into from
Sep 30, 2022
Merged

Migrate to hatch #1197

merged 18 commits into from
Sep 30, 2022

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Sep 6, 2022

References

Kickstarting the migration to hatch using the migration script: https://hatch-jupyter-builder.readthedocs.io/en/stable/source/how_to_guides/migrating_javascript_projects.html

pip install hatch_jupyter_builder
python -m hatch_jupyter_builder.migrate .

More info: https://blog.jupyter.org/packaging-for-jupyter-in-2022-c7be64c38926

Code changes

  • Update to hatch.
  • Use hatch to bump the Python version
  • Handle CSS files downloaded in setup.py:

voila/setup.py

Lines 112 to 118 in b16d377

def download_css_first(command):
class CSSFirst(command):
def run(self):
self.distribution.run_command("download_css")
return command.run(self)
return CSSFirst

User-facing changes

None

Backwards-incompatible changes

None

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

Binder 👈 Launch a Binder on branch jtpio/voila/hatch

@jtpio
Copy link
Member Author

jtpio commented Sep 8, 2022

Getting the following error for the UI tests:

WARNING: voila 0.3.6 does not provide the extra 'visual_test'

Seems to be having troubles installing the visual_test optional-dependencies with python -m pip install --upgrade ".[test,visual_test]"

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 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 93 <- [102 - 116 - 140] -> 198 75 <- [76 - 82 - 97] -> 130 80 <- [86 - 90 - 102] -> 132 76 <- [80 - 86 - 99] -> 123 2731 <- [2795 - 2846 - 2933] -> 3832 2635 <- [2650 - 2684 - 2698] -> 3001 2838 <- [2864 - 2877 - 2879] -> 3262 3208 <- [3245 - 3255 - 3391] -> 3650 2113 <- [2135 - 2139 - 2201] -> 2460 3753 <- [3879 - 3979 - 4103] -> 4532 10420 <- [10551 - 10973 - 11564] -> 13434 7978 <- [8037 - 8039 - 8055] -> 8360 1445 <- [1522 - 1577 - 1587] -> 1665 2649 <- [2658 - 2667 - 2809] -> 2823
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-09-29 14:13:33.322245086 +0000
+++ /dev/fd/62	2022-09-29 14:13:33.322245086 +0000
@@ -4,7 +4,7 @@
     "BENCHMARK_REFERENCE": "actual"
   },
   "browsers": {
-    "chromium": "97.0.4666.0"
+    "chromium": "94.0.4595.0"
   },
   "systemInformation": {
     "cpu": {
@@ -34,7 +34,7 @@
       "voltage": ""
     },
     "mem": {
-      "total": 7281311744
+      "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
     }

@jtpio
Copy link
Member Author

jtpio commented Sep 8, 2022

Seems to be having troubles installing the visual_test optional-dependencies with python -m pip install --upgrade ".[test,visual_test]"

So 55ab7ed seems to be fixing it, by updating visual_test to visual-test.

@jtpio
Copy link
Member Author

jtpio commented Sep 8, 2022

CI is now passing and it is also looking good on Binder, marking as ready.

image

@jtpio jtpio marked this pull request as ready for review September 8, 2022 11:36
@jtpio
Copy link
Member Author

jtpio commented Sep 8, 2022

cc @martinRenou @trungleduc for review.

I think it should be fine to get this in a 0.3.x release. Although we could also ship that in a 0.4.0 release with #1046.

@martinRenou
Copy link
Member

I took the liberty to rebase and fix the conflict

setup.py Show resolved Hide resolved
@jtpio jtpio marked this pull request as draft September 26, 2022 15:45
@jtpio
Copy link
Member Author

jtpio commented Sep 26, 2022

Converting back to a draft for now to check the handling of extra CSS files that used to happen in setup.py.

@jtpio
Copy link
Member Author

jtpio commented Sep 29, 2022

The CSS files now seem to be correctly downloaded and available when looking at the releaser assets from https://github.com/voila-dashboards/voila/actions/runs/3150560686

image

@jtpio
Copy link
Member Author

jtpio commented Sep 29, 2022

The size of the sdist seems to be bigger than before:

image

Which might be related to the gif that are included:

image

These gifs are not in the assets on the latest main: https://github.com/voila-dashboards/voila/actions/runs/3150516185

So we should be able to filter them out.

@jtpio jtpio marked this pull request as ready for review September 29, 2022 12:25
@jtpio
Copy link
Member Author

jtpio commented Sep 29, 2022

Marking as ready for a second round of review.

The UI tests failure might not be related. The "Expected" snapshot currently looks broken and might need to be updated separately (or in this PR):

https://github.com/voila-dashboards/voila/blob/main/ui-tests/tests/voila.test.ts-snapshots/basics-classic-linux.png

ui-tests-mismatch.mp4

pyproject.toml Outdated Show resolved Hide resolved
@jtpio
Copy link
Member Author

jtpio commented Sep 29, 2022

Please update galata references

@jtpio
Copy link
Member Author

jtpio commented Sep 29, 2022

The snapshot update for the classic template in 5cd10ea looks legit. Not sure about reveal though.

@jtpio jtpio closed this Sep 29, 2022
@jtpio jtpio reopened this Sep 29, 2022
@martinRenou
Copy link
Member

Looking at another build like for this PR, the UI-tests seems to pass. So I'm not sure I understand why this PR would need to change the reference screenshots?

@jtpio
Copy link
Member Author

jtpio commented Sep 30, 2022

Agree it's odd. One thing for sure is the current classic snapshot on main which does not look correct (#1197 (comment)).

Wondering if this switch to hatch fixes some things to the way Voila is built and packaged for the UI tests.

@martinRenou
Copy link
Member

That could be it indeed.

Your PR looks good to me. Let's probably move forward and merge?

@martinRenou martinRenou merged commit d210b62 into voila-dashboards:main Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants