build: refactor build strategy

[12rambau]

W.I.P but should fix #256

• move everything I can to pyproject.toml as it has won the configuration war
• remove warning, raise etc... from setup..py as they are ignored since setuptools 50
• use an python package for NPM binding removing many extra line from the setup.py page

I clearley identify the from generate_source import generate as the culprit but I don't yet manage to find a way to load it BEFORE the end of the installation process. It seems to be unavailable in the isolated environment even if it's part of the egg_info folder.

TODO

• solve the installation issue
• check that all data files are still there

[12rambau]

I think what we try to achieve here (igenerate the source upon installation) is not possible when the lib is build in isolation as the generate_source module is not yet available in the venv.

Maybe an alternative solution could be:

• for release:
  • generate the source
  • generate the wheel (with the sources inside)
  • publish
• for local build:
  • create a nox session (build: use nox for tests and docs  #240) to generate the sources for local dev.
  • explain the process in the contribute section of the doc

@mariobuikhuizen, @maartenbreddels what do you think ?

[maartenbreddels]

not ideal, i've always though a pip install should work. Why doesn't the isolated build pick up the generate source code, especially since it is in the manifest file, it should be available (maybe in a different directory.. ?)

[maartenbreddels]

Did you rebase to get 778437a in?

[12rambau]

I don't remember rebasing but it's already in. I'm discovering the deps of setuptools with pyca developers, the pip install now works but the python -m build doesn't.

pypa/setuptools#3909 (reply in thread)

[12rambau]

@maartenbreddels, I'm moving forward with the installation process. Still few things to optimize but at least it works (pip install, pip install -e and python build).

Things are still buggy in the tests though. I have several questions:

• why is there a build job ? the produced artifacts are reused in all the others but they exactly correspond to the one produced by a python build, that could be relaunched (not that long) for every job and thus make them in parralel.
• why not installing the lib the same way people are doing it with pip install .. refering to the build action artifact is the main reason last version was released with a non complete wheel. I think we should only rely on pip install for tests.
• why not integrating the solara based test in a pytest suit ? so that it can be launched by developer before pushing ? I think it can be managed after build: use nox for tests and docs  #240

If you agree with my comments I'll move forward with my PR and revamp the test actions, the release one will deserve a dedicated PR.

[maartenbreddels]

I'm moving forward with the installation process. Still few things to optimize but at least it works (pip install, pip install -e and python build).

Great news!

• why is there a build job ?

I want the build step to be isolated, as to mimic as much as possible a release->user install process. We don't want that because of some artifact during build process, our tests succeed, but when installing the produced wheel it will not (this in fact caused a faulty release).

• the produced artifacts are reused in all the others but they exactly correspond to the one produced by a python build

Yes, that's exactly what we want to test: Do the tests pass when the wheel that we build is released as it is. We don't "care" about the tests passing because we somehow managed to get it working.

• why not installing the lib the same way people are doing it with pip install .

No user installs it this way right? It's all comping from pypi, which means people install the wheel.

• why not integrating the solara based test in a pytest suit

Not sure what you mean by this? It's in the test directory right, if you run py.test tests it will run every test locally. The reason we split it, is because the UI tests are pretty expensive (long to run), and we don't need to test it for every Python version. Does that make sense?

[maartenbreddels]

PS: I'd rather have you rebase on master, then merge master back in, this is often confusing (at least for me) and leads to weird incomprehensible conflicts (again, at least for me:P)

[12rambau]

No user installs it this way right? It's all comping from pypi, which means people install the wheel.

if you run pip install . on a project that build a universal wheel with a pyproject.toml then the wheel is produced (at list its content in sdist) and used for the installation so that actually mimick more the installation from pypi than the current one. On the plus side you check if you included all the necessary requirements.

PS: I'd rather have you rebase on master, then merge master back in, this is often confusing (at least for me) and leads to weird incomprehensible conflicts (again, at least for me:P)

I know I'll try to make the test pass first and then rebase (which will be a pain for sure)

[maartenbreddels]

I will believe that pip install . should behave the same, but I think having a different build and test environment makes sure that you reproduce exactly how users run it.
Ideally, we also check the pip install . and pip install -e . path. But I think that will not affect end-users, so it's less important. Most important I think is to never break a user-install.

[12rambau]

I reintroduced a build step (I think it's a duplicate but beter safe than sorry), dropped 3.6 and all the test are passing. The solara based ui assesment is not working and as I have 0 knowledge, could you have a look ?

side note: I'm not 100% sure we need to check ui aspect of any widget as it will be displayed in the documentation anyway using jupyterlite.

[maartenbreddels]

I'm not 100% sure we need to check ui aspect of any widget as it will be displayed in the documentation anyway

This makes sure we can never have a broken release. If the UI tests pass, we know it's working on classic notebook, lab, voila and solara.

Why did you drop 3.6? We still need to support it. Are we using a non-supported feature in this PR?

[maartenbreddels]

We cannot see what is going wrong now, take a look at my comments so we can debug this.

[12rambau]

Let me advocated on some of your suggestions:

why don't I want to rely on the build wheel ?

Because it's strictly equivalent to running pip install . when using setuptools (confirmed in the thread where I talk with them) so building the wheel manually to save it and share it on each runner is making the actions more complex -> more dificult to maintain. The current matrix is only running on ubuntu but we could extend it to OSX and windows, it's much more dificult to share files between platform which would force to special case for windows. Finally for the tests, it's super easy to add the test requirements in extras (which is the case in this PR) which is not possible using the wheel. this what is done on all the repositories I'm working on and many of them include nodeJS deps, we never encountered any issue (https://github.com/pydata/pydata-sphinx-theme). I had the feeling that the number of concurrent jobs is fix so if they run all at once I'm still a bit faster.

Why do I think the solara test is overdoing it ?

I would like to use mystnb in the documentation which will build ALL the widgets. using pytest-regression, it will be super easy to check the produced html widgets. It will be equivalent to the test performed on solara without using playwrite which can be tricky (we are using it in pydata-sphinx-theme for a11y and that's is bugging on runners 1 time out of 2) and ensurin the test of all of them.

why do I want to drop 3.6 ?

According to python documentation, 3.6 have reached end-of-life for a long time now and has been dropped by multiple other libs.

https://devguide.python.org/versions/

[12rambau]

everything seems ok now !

[maartenbreddels]

😍

[12rambau]

@maartenbreddels did you had the time to have a look at the latest iteration ?

[maartenbreddels]

I've taken a quick look, it looks nice!
So cmdclass={"egg_info": js_prerelease(egg_info)} is the magic trick?

[12rambau]

it seems it covers all cases as any build type requires to create a egg_info folder first.

[12rambau]

I was crowling in my notification dashboard, and I saw this was still requesting changes. @maartenbreddels is it a legacy request or should I change anything ?

[mariobuikhuizen]

Thank you very much for fixing the build and sorry for the long delay.

Please try do fixes, adding linting, refactoring, etc in separate PR's next time. This makes them quicker/easier to review, discuss and merge.

A few small issues:

[mariobuikhuizen]

Why remove python 3.6? Our experience is that some users are still tied to 3.6 because of other dependencies or slow upgrade cycles in companies.

[12rambau]

As 3.7 as already reached end of life, keeping the previous one that is already dead seems too much for me. if you would like to keep it please run the test again as I may have introduce some pathlib mechanism that were not available in 3.6

[mariobuikhuizen]

Sadly the reality is that 3.6 it's not dead, especially in big companies.

[12rambau]

solved in f3980bb

[12rambau]

it seems pytest-playwright is not compatible with 3.6 either, making the build fail. what should I do ?

[mariobuikhuizen]

I think we only need solara[pytest] and thus ${wheel}[test] in the ui-test job. Removing the [test] from $(find dist -name *.whl)[test] will fix it.

[12rambau]

First I would like to solve the 3.6 build. all the others from 3.7 to 3.11 worked but 3.6 failed because playwright is not working. I cannot drop [test] there as it's where the test requirements are but if they are based on playwright it seems my life will become more complicated

[mariobuikhuizen]

I think you removed [test] from the ui-test instead of the test job.

[12rambau]

you were talking about the ui-test so I checked. I cannot remove [test] from the normal test as solara itself is set in it.

Just to advocate again in favor of dropping python 3.6, many of the top used widget libs that I know (+other packages that are not widgets) have already dropped it:

• ipywidgets: https://pypi.org/project/ipywidgets/ >=3.7
• ipyleaflet: https://pypi.org/project/ipyleaflet/ >= 3.7
• matplotlib: https://pypi.org/project/matplotlib/ >= 3.8
• pandas: https://pypi.org/project/pandas/ >= 3.8

To make the test work on 3.6 we should redesign them without use of playwright which is use directly as a fixture.

At the end of the day it will continue to work with 3.6 we will just stop making bug fixes.

[mariobuikhuizen]

It's fixed with removing [test] in the test job. We can postpone dropping 3.6 support.

[mariobuikhuizen]

Should we squash and merge this?

[12rambau]

As you see fit, I personnaly don't mind. It will ease readability in main though