-
Notifications
You must be signed in to change notification settings - Fork 225
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 to hatch-based build system and fix CI #541
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for working on this!
Would be nice to setup the jupyter-releaser after that
"Programming Language :: Python :: 3.8", | ||
"Programming Language :: Python :: 3.9", | ||
"Programming Language :: Python :: 3.10", | ||
"Programming Language :: Python :: 3.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add Python 3.12 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I don't know why I stopped at 3.11.
We should probably decide on what Python versions we are supporting, heading toward SPEC0 eventually. Maybe 3.9 to 3.12 is appropriate now. Then I could ensure that CI is running using both the earliest and latest Python versions that we claim to support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍🏽
I'll take a look at jupyter-releaser. |
Bot update galata references |
The bot is failing for some other reason. Anyway I believe the CI failures are unrelated. It seems the visual regression diff is super small and probably due to some update on the matplotlib side. I can look into getting this green separately. |
I am going to look at excluding text from the Matplotlib plots. This is done in Matplotlib CI as there can be tiny differences depending on platform, OS and compiler. |
Excluding text from the Matplotlib test images looks successful so far, just need to wait for the last 2 CI runs. Things left to do in future PRs:
In other words look at all github actions other than |
Thank you! I guess this PR is good to go as-is!
The fix could probably be just https://github.com/jupytercad/jupytercad/blob/main/.github/workflows/update_galata_references.yaml#L31 |
dist: 'ipympl*.whl' | ||
# Install from sdist not wheel | ||
- python: '3.12' | ||
mpl-version: 'latest' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we missing an os
entry here? I wonder if this is the reason why the workflow does not start
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I've corrected this now.
Could you cancel the old CI runs as I don't have permissions to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
96080eb
to
8763432
Compare
I changed the test that installed from sdist from python 3.12 to 3.11 so now it passes. With python 3.12 it was trying to compile zmq from source. I don't think that should be necessary, but it is not a problem for us to solve today. |
We may be able to workaround this issue by installing from source with Thank you for getting this green! Merging and we can iterate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
This PR switches the build system from
jupyter_packaging
to the recommendedhatch_jupyter_builder
, plus some other smaller changes, in an attempt to get CI passing. When it succeeds and this PR is merged, I will deal with other improvements such as bringing dependencies up to date.Summary of changes made:
hatch
-based builder.micromamba
github actions.python -m build
to build the package rather than the deprecatedpython setup.py ...
jlpm
rather thanyarn
for TypeScript package management and building.With these changes I can build and run the tests locally, and also install from the generated sdist and binary wheel.