FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Update to hatch-based build system and fix CI by ianthomas23 · Pull Request #541 · matplotlib/ipympl · GitHub

/ ipympl Public

Update to hatch-based build system and fix CI - #541

Merged
martinRenou merged 6 commits into
matplotlib:mainfrom
ianthomas23:hatch_and_ci
Jan 24, 2024
ConversationCommits (6)ChecksFiles changed
Merged

Update to hatch-based build system and fix CI#541
martinRenou merged 6 commits into
matplotlib:mainfrom
ianthomas23:hatch_and_ci

Conversation

ianthomas23 commented Jan 23, 2024

Copy link
Copy Markdown
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.

github-actions Bot commented Jan 23, 2024

Copy link
Copy Markdown
Contributor

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

martinRenou reviewed Jan 23, 2024
View reviewed changes

martinRenou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM! Thanks for working on this!

Would be nice to setup the jupyter-releaser after that

Comment thread pyproject.toml
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",

martinRenou Jan 23, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should we add Python 3.12 here?

ianthomas23 Jan 23, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

martinRenou Jan 23, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sounds good 👍🏽

ianthomas23 commented Jan 23, 2024

Copy link
Copy Markdown
Member Author

Would be nice to setup the jupyter-releaser after that

I'll take a look at jupyter-releaser.

martinRenou commented Jan 23, 2024

Copy link
Copy Markdown
Member

Bot update galata references

martinRenou commented Jan 23, 2024

Copy link
Copy Markdown
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 commented Jan 23, 2024

Copy link
Copy Markdown
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 commented Jan 23, 2024

Copy link
Copy Markdown
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 commented Jan 23, 2024

Copy link
Copy Markdown
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

martinRenou reviewed Jan 24, 2024
View reviewed changes
Comment thread .github/workflows/main.yml
dist: 'ipympl*.whl'
# Install from sdist not wheel
- python: '3.12'
mpl-version: 'latest'

martinRenou Jan 24, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Are we missing an os entry here? I wonder if this is the reason why the workflow does not start

ianthomas23 Jan 24, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, I've corrected this now.

Could you cancel the old CI runs as I don't have permissions to do that?

ianthomas23 Jan 24, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks!

ianthomas23 commented Jan 24, 2024

Copy link
Copy Markdown
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 commented Jan 24, 2024

Copy link
Copy Markdown
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

martinRenou approved these changes Jan 24, 2024
View reviewed changes

martinRenou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you!

martinRenou merged commit c218e07 into matplotlib:main Jan 24, 2024
ianthomas23 deleted the hatch_and_ci branch January 24, 2024 11:13
ianthomas23 mentioned this pull request Mar 15, 2024
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

martinRenou martinRenou approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL