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 to hatch-based build system and fix CI #541

Merged
merged 6 commits into from Jan 24, 2024

Conversation

ianthomas23
Copy link
Member

This PR switches the build system from jupyter_packaging to the recommended hatch_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:

  • Switch to hatch-based builder.
  • Update micromamba github actions.
  • Use python -m build to build the package rather than the deprecated python setup.py ...
  • Use jlpm rather than yarn 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.

Copy link
Contributor

Binder 👈 Launch a binder notebook on branch ianthomas23/ipympl/hatch_and_ci

Copy link
Member

@martinRenou martinRenou left a 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",
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good 👍🏽

@ianthomas23
Copy link
Member Author

Would be nice to setup the jupyter-releaser after that

I'll take a look at jupyter-releaser.

@martinRenou
Copy link
Member

Bot update galata references

@martinRenou
Copy link
Member

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.

@ianthomas23
Copy link
Member Author

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.

@ianthomas23
Copy link
Member Author

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:

  • Look into jupyter-releaser
  • Fix update_galata_references.yaml

In other words look at all github actions other than main.yml.

@martinRenou
Copy link
Member

Thank you! I guess this PR is good to go as-is!

Fix update_galata_references.yaml

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'
Copy link
Member

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

Copy link
Member Author

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

@ianthomas23
Copy link
Member Author

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.

@martinRenou
Copy link
Member

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 --no-deps and making sure all dependencies are installed using conda/mamba from conda-forge.

Thank you for getting this green! Merging and we can iterate

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Thank you!

@martinRenou martinRenou merged commit c218e07 into matplotlib:main Jan 24, 2024
9 checks passed
@ianthomas23 ianthomas23 deleted the hatch_and_ci branch January 24, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants