-
Notifications
You must be signed in to change notification settings - Fork 18
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
UV Package Manager Handles Dev Installation Much Faster #355
Conversation
b943a98
to
f88317b
Compare
Thanks for the contribution! I personally use Do QuantStack folks use/like |
100% agree! It is not just about speed; handling a variety of other stuff so well is so enjoyable (python exec. is marvelous)! Looking back, my PR description feels a lil faky, but I believe I meant to demonstrate professionalism! Anyway, I would be pleased to get some comments / suggestions on what to change next if the QuantStack folks are also in!/ Cheers |
Not sure why some checks aren't running. Trying to re-trigger :) |
I believe. If I "un" delete the requirements-build.txt, it will go through, but does this imply that I may also need to touch the Github Workflows? I have not checked the logs, however this occurred when I amended my lone commit by removing the build requirements txt file (which is no longer required if the migration to UV comes to light). Thoughts? |
Not at all, I think you did a great job. Very detailed, you shared your thought process, and you used a very friendly tone. Thanks for that :)
Looks like you're in Eastern US time zone; they will likely get back to us while we're sleeping. QuantStack is centered around Central European Time! |
The thing I don't get is the workflows aren't even starting. I've had this happen before and usually closing and re-opening PRs fixes it, presumably because GitHub infrastructure had a hiccup. The workflows will need to be changed, but they should have run and thrown an error instead of not run at all. I'm confused. |
There are some merge conflicts that are preventing workflow from running |
I thought GitHub would still run workflows in that case, just from the tip commit of the branch. I am clearly mistaken, thanks @arjxn-py 🙇 |
@simonprovost are you comfortable rebasing? |
f88317b
to
ed51539
Compare
Thanks @mfisher87 – Very much appreciated!
Sure thing! Let's wait it's not big deal anyway :)
Thanks @arjxn-py ! Indeed I saw it then obviously did not check if some stuff was pushing ahead on the Rebase done + some docs update ✅ EDIT 1.0: There is well an issue with the fact that requirements-build.txt is now deleted :) Should I be in charge of modifying the workflows as part of the PR or not? Given it has a much broader scope. I could have a shot at it. |
I do think this PR should fix any workflows it breaks, but we can also help with that! What do you prefer? |
Let me have a look! There is a lot but I had a thoughts earlier! Should provide a commit soon. |
ed51539
to
c97ddfb
Compare
5017963
to
1a3b884
Compare
b4f726d
to
3352e7c
Compare
for more information, see https://pre-commit.ci
@mfisher87 or @arjxn-py May I get approval to run the read the doc again please ? I am getting frustrated by the errors :) |
Don't forget we need the rest of the JupyterGIS team to be comfortable with a switch to UV, so please don't get too invested too soon :) |
Thanks a lot for your interest in JupyterGIS and making a PR!
We actually don't :) We use micromamba, which is essentially the same but:
UV looks great for Python users, but we're not only using Python here. I am personally not feeling confortable with this PR. The diff looks like it's adding quite a lot of complexity, and I'm not sure there is a real benefit here. Most of the slowness in the dev setup/CI comes from the Webpack/TypeScript setup. We could speed up our CI aggressively by adopting e.g. esbuild or rspack instead of webpack, but this is not a choice we can make at the JupyterGIS level, we need to wait for JupyterLab to make such move.
Sorry to be that guy who doubts on your testing, but are you sure the TypeScript/Webpack steps have not been cached in the meantime? 20 seconds looks like you were not actually building the JavaScript assets. I'm sorry you spend too much time on this PR already. IMO we should not go ahead with this. On the long term we would need to maintain this and I don't want to be messing around with a mix of micromamba and uv on my machine. |
Seconding Martin here: language agnosticism is a key point here. Being able to install QGIS, or R, is really important, as we will probably offer an R API similar to the Python API. Another key point is that the combination of Micromamba and Jupyter is also central in our JupyterLite deployment story, based on emscripten-forge. |
Hi @martinRenou @SylvainCorlay , Do not worry! This makes sense (i.e. language agnosticism). However, certain packages appear to be "hidden" in scripts, which is not as effective as a package management like UV, Poetry, or any other more pythonic-focused tool that centralise stuff in their pyproject which is much more maintainable IMHO. Maybe Micromamba does, but let us not dig deeper it has not high value. Cheers, |
Thanks so much for your contribution and understanding, @simonprovost :)
I think we can move in this direction in smaller steps without changing tooling, and that'd be really valuable! |
Description
Hiya folks!
When installing JupyterGIS from source, I noticed that even with a reliable university Wi-Fi, the installation process is quite slow. Roughly 3 long minutes with a recent laptop config and a neat university's WI-FI. Considering the infancy of the initiative, I believe adopting UV as the package manager internally could greatly enhance scalability for both the core team and contributors on the long term 🎉
Why UV?
Happy Ruff they say!
🚴UV’s official documentation highlights its faster handling of packages and installations compared to alternatives. Below is an example that illustrates UV's performance advantages:
Furthermore, you guys already using
Ruff
, why not entirely useAstral
's capabilities 🌂☀️ Key Changes in This Pull Request
Updated the
pyproject.toml
file:pyproject.toml
file (i.e Included build and development-focused packages directly within this file for clarity).dev-install.py
.Retained the
dev-install.py
file:Updated
build_packages.py
:👀 Performance Improvements (
dev-install.py
only! build packages is not much of interest to improve anyway):Using my MacBook Pro M2 (13"), and a Eduroam University WI-FI spot, I observed a non-neglieable reduction in building-from-source time:
Roughly 90% improvement is not negligible for the time being to ensure scalability in this direction, right?
Post-Installation Testing
I verified that all existing JupyterGIS example files work as expected after installation. They appear consistent with the latest version from the main branch.
Could I run the unit-tests? May I get the command line? Is there anything more thorough I could test ? because what I have done is pretty much just playing around the pot.
I’m happy to hear your feedback or suggestions! I hope these changes help improve scalability and ease of contribution in the future.
Cheers,
📚 Documentation preview: https://jupytergis--355.org.readthedocs.build/en/355/
💡 JupyterLite preview is available from the doc, by clicking on