Skip to content

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

EstebanGZam
Copy link

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:

  1. Project Modernization

    • Migrated from setup.py to pyproject.toml with all metadata preserved
    • Generated uv.lock for reproducible environments
    • Added clear documentation in README.md for UV usage
  2. New Workflow Features

    # 1. Setup environment
    uv venv venv
    source venv/bin/activate
    
    # 2. Install with all dependencies
    uv pip install -e .
    uv pip install -e ".[dev,test]"
    
    # 3. Run tests
    uv pip install coverage
    uv run --active coverage run -m unittest discover -s tests -p 'test_*.py'
    uv run --active coverage report -m

Benefits:

  • Faster onboarding: New contributors can setup environments in seconds
  • 📦 Consistent installations: Precise dependency resolution avoids "works on my machine" issues
  • 🧪 Efficient testing: uv run executes tests with minimal overhead

Verification:

  • Confirmed all unit tests pass with the new UV workflow
  • Verified coverage reporting works as expected

Resolves #64

@VigneshVSV VigneshVSV self-requested a review April 15, 2025 06:42
@VigneshVSV
Copy link
Collaborator

Hi, I will get to this in a day or two max.

@VigneshVSV
Copy link
Collaborator

I would add this:

Benefits:
⚡ Faster onboarding: New contributors can setup environments in seconds
📦 Consistent installations: Precise dependency resolution avoids "works on my machine" issues
🧪 Efficient testing: uv run executes tests with minimal overhead

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 pyproject.toml and the __init__.py within hololinked folder.

@VigneshVSV VigneshVSV self-assigned this Apr 15, 2025
@VigneshVSV VigneshVSV moved this to In review in hololinked Apr 15, 2025

[project.urls]
Documentation = "https://hololinked.readthedocs.io/en/latest/index.html"
Repository = "https://github.com/your-username/hololinked" # Update with actual repository URL
Copy link
Collaborator

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

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

Copy link
Collaborator

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/
Copy link
Collaborator

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"
Copy link
Collaborator

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",
Copy link
Collaborator

@VigneshVSV VigneshVSV Apr 15, 2025

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.

Copy link
Collaborator

@VigneshVSV VigneshVSV Apr 16, 2025

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.

Copy link
Collaborator

@VigneshVSV VigneshVSV Apr 18, 2025

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.

@EstebanGZam
Copy link
Author

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:

"if examples are supported, that is even better (at least for the simulations)"

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:

  1. Open a new issue dedicated exclusively to this feature.
  2. Define the implementation details there (optional dependencies, expected structure, etc.).
  3. Work on it as the next priority.

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.

@VigneshVSV
Copy link
Collaborator

VigneshVSV commented Apr 16, 2025

  1. Open a new issue dedicated exclusively to this feature.
  2. Define the implementation details there (optional dependencies, expected structure, etc.).
  3. Work on it as the next priority.

This way, we can manage example dependencies in a more organized manner and with clear expectations.

Agree! Lets just keep this for another issue. I also worked on something in uv last week at my workplace and things were not so easy. I will open a new issue coming weekend.

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

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"
]
Copy link
Collaborator

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.

@VigneshVSV
Copy link
Collaborator

VigneshVSV commented Apr 18, 2025

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.

@VigneshVSV
Copy link
Collaborator

VigneshVSV commented Apr 19, 2025

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:

git push -f

If you need more help, just let me know. There are only two commits, so it should be fairly straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

setup uv python package and environment management
2 participants