-
Notifications
You must be signed in to change notification settings - Fork 0
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
Migrate project to uv #99
base: master
Are you sure you want to change the base?
Conversation
pyproject.toml
Outdated
[project] | ||
name = "lundpython" | ||
version = "0.1.0" | ||
description = "Add your description 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.
I wouldn't keep the default here.
requires-python = ">=3.12" | ||
dependencies = [ | ||
"astropy>=7.0.0", | ||
"jupyter>=1.1.1", | ||
"jupyterlab-rise>=0.42.0", | ||
"matplotlib>=3.9.2", | ||
"numpy>=2.1.3", | ||
] |
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.
Is there a good reason for not complying with SPEC 0?
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.
What part of this is in violation of SPEC 0?
It seems to me the spec is about not supporting or requiring old versions of packages, and all these are new.
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.
Specifying how long specific versions of dependencies should be supported is the entire point of the SPEC:
Specifically, we recommend that:
- Support for Python versions be dropped 3 years after their initial release.
- Support for core package dependencies be dropped 2 years after their initial release.
Although here it makes sense to support versions that will be 2 years old next autumn instead of right now.
|
||
[dependency-groups] | ||
dev = [ | ||
"pre-commit>=4.0.1", |
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.
I wouldn't write down pre-commit
as a dependency here. On one hand, listing it as a dependency is not good enough because that just installs the framework, but the hooks still need to be installed in a separate manual step anyways. On the other hand, there aren't any benefits to isolating pre-commit
because the framework sets up separate environments for each of the hooks itself.
dev = [ | ||
"pre-commit>=4.0.1", | ||
] | ||
tools = [ |
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.
Add a one-sentence comment to explain why these are separate.
uv.lock
Outdated
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.
What's the purpose of locking down the dependencies? Our instructions should not critically depend on exact versions students might have installed. It's not at all obvious why specifying the minimal supported versions on pyproject.toml
would not be good enough.
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.
I included the lock file because that is the default recommended behaviour of uv, but I agree that we don't require that strict control so it is better to leave it out.
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.
I'm willing to guess that the uv
recommendation relies on the assumption that the code is tested using different versions of dependencies. But this project doesn't even have a test suite, so if a lock file forces the few people who are involved with this to use exactly the same versions of dependencies then some problems might go unnoticed for a long time.
Also update pre-commit hooks versions.
It has been replaced with pyproject.toml
Add a pyproject.toml with project dependencies.
Dependencies are now in three groups.
Notably, the original RISE is not included because it is not compatible with JupyterLab or Jupyter notebook > 6. Instead, we use jupyterlab-rise. This has its own issues, most notably that when executing the last cell on a slide, the presentation will advance. I have tried the workaround proposed here which is to insert an empty cell of type
skip
at the end of each affected slide. I found that it works consistently but another PR will probably be needed to add these to all slides since the issue does not seem to be worked on.Remove Pipfile and add uv.lock.
I also updated the pre-commit hooks since I was messing with it anyway.