-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add UV support for faster development workflows #70
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
base: main
Are you sure you want to change the base?
Conversation
Hi, I will get to this in a day or two max. |
I would add this: Benefits: to changelog.md. Nice emojis. Also can you bump the version to 0.2.11? I will do a test deployment to testpypi.org The files to change are the |
|
||
[project.urls] | ||
Documentation = "https://hololinked.readthedocs.io/en/latest/index.html" | ||
Repository = "https://github.com/your-username/hololinked" # Update with actual repository URL |
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.
|
||
```bash | ||
uv pip install coverage |
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.
can you add coverage as a test dependency?
|
||
##### Setup Development Environment | ||
|
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 would just say "One can setup a development environment with uv as follows:" and also remove "##### Setup Development Environment"
##### Setup Development Environment | ||
|
||
1. Install uv if you don't have it already: https://docs.astral.sh/uv/getting-started/installation/ |
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.
Also no need for this line. I plan to add one more section in the README, which is why I wish to remove unnecessary lines.
"pandas==2.2.3", | ||
"pydantic>=2.8.0,<3.0.0", | ||
"serpent==1.41", | ||
"uvloop==0.20.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.
Windows does not support uvloop. I would remove this dependency as uv pip install -e ".[dev,test]
is not working in windows.
"argon2-cffi>=23.0.0", | ||
"ifaddr>=0.2.0", | ||
"msgspec>=0.18.6", | ||
"pyzmq>=25.1.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.
I need a day or two time to check this ZMQ dependency. Something seems to be broken in windows, although I have conda environments which are working correctly. In linux it seems to be working as seen in the test jobs in github.
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.
Update: when installing this dependency list with pip
, say uv pip install pip
, python -m pip install .
, we receive the version for ZMQ pyzmq-26.4.0-cp312-cp312-win_amd64.whl
, which works. Only the version installed by uv
is breaking in windows.
Above not true - ZMQ version 26.4 has IPC disabled, although supported. See issue here: zeromq/pyzmq#1981 and changelog here: https://pyzmq.readthedocs.io/en/latest/changelog.html (search IPC, version 26.2 has it disabled)
We need to install lower versions.
Still working on it.
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.
"pyzmq>=25.1.0,<26.2" solves the problem
Ran 40 tests in 79.639s
OK
(Windows)
I have created an issue for adding windows runners in github CI - #71. but if you constrain the ZMQ version, its sufficient for this merge request.
I understand. I'll start working on these revisions in the next few days. Regarding the section on support for simulation examples mentioned in the issue:
I acknowledge that I haven't been able to implement this part as I would need more clarity on the specific requirements. To ensure proper implementation, I propose:
This way, we can manage example dependencies in a more organized manner and with clear expectations. What do you think of this approach? I'm open to adjusting it based on what you consider most appropriate. |
Agree! Lets just keep this for another issue. I also worked on something in |
coverage run -m unittest discover -s tests -p 'test_*.py' | ||
coverage report -m | ||
source .venv/bin/activate | ||
uv run coverage run -m unittest discover -s tests -p 'test_*.py' |
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.
Known from before, not specific to MR, but this exact command does not work in windows terminal. I used to do python -m unittest
instead. If you know a solution, feel free to add it to README or just resolve the comment without a solution.
"requests==2.32.3", | ||
"numpy>=2.0.0", | ||
"pydantic>=2.8.0,<3.0.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.
There is a requirements.txt
within the test folder. I prefer to use what is specified here and that file can be deleted.
Hi @EstebanGZam, I have completed my comments, sorry the delays. There is nothing more to add from my side. Thanks again for this merge request. |
I merged this pull request: #73 You need to git checkout main
git pull
git checkout <this branch>
git rebase main If conflicts occur while rebasing, choose which changes to keep, then git add <file names that were resolved>
git rebase --continue finally, when rebase finishes, which you will see in git status:
If you need more help, just let me know. There are only two commits, so it should be fairly straightforward. |
This PR implements UV as the primary package manager for the project, addressing issue #64. UV delivers significant improvements in dependency resolution speed (10-100x faster than pip) and simplifies local development setup.
Key Changes:
Project Modernization
setup.py
topyproject.toml
with all metadata preserveduv.lock
for reproducible environmentsNew Workflow Features
Benefits:
uv run
executes tests with minimal overheadVerification:
Resolves #64