Skip to content

Maint/deprecate requirements txt#467

Merged
rugeli merged 10 commits intomainfrom
maint/deprecate-requirements-txt
May 6, 2026
Merged

Maint/deprecate requirements txt#467
rugeli merged 10 commits intomainfrom
maint/deprecate-requirements-txt

Conversation

@rugeli
Copy link
Copy Markdown
Collaborator

@rugeli rugeli commented Apr 27, 2026

Problem

closes #459

Solution

  • removed the per-platform requirements.txt files and the workflow that generates them
  • updated Dockerfile.ecs to use uv sync against uv.lock, so dependency versions stay pinned
  • added a new [dependency-groups].server = ["aiohttp"] group to declare the web-server dependency
  • updated documentation

Type of change

  • Maintenance (non-breaking change which preserves existing functionality)

Steps to Verify:

  1. pull the branch and run uv sync --group server, it should resolve and install aiohttp along with the rest of the deps from uv.lock
  2. (optional) rundocker build -f docker/Dockerfile.ecs -t cellpack-ecs, the image should build
  3. (optional) rundocker run -p 80:80 cellpack-ecs then go to http://0.0.0.0:80/hello, should return Hello from the cellPACK server

@github-actions
Copy link
Copy Markdown
Contributor

Packing analysis report

Analysis for packing results located at cellpack/tests/outputs/test_spheres/spheresSST

Ingredient name Encapsulating radius Average number packed
ext_A 25 236.0

Packing image

Packing image

Distance analysis

Expected minimum distance: 50.00
Actual minimum distance: 50.01

Ingredient key Pairwise distance distribution
ext_A Distance distribution ext_A

Comment thread docker/Dockerfile.ecs
Comment on lines +7 to +9
RUN python -m pip install uv --root-user-action=ignore
RUN uv sync --no-default-groups --extra server --frozen
ENV PATH="/cellpack/.venv/bin:$PATH"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

initially I used pip install ".[server]", but that resolves fresh from PyPI on each build which means rebuilds could pick up different versions. Switching to uv sync keeps the install pinned to uv.lock, giving us more consistent builds.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just tried running this locally and it worked perfectly!

@rugeli rugeli requested a review from Copilot April 27, 2026 22:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes legacy, autogenerated per-platform requirements.txt artifacts in favor of using uv.lock/uv sync for pinned dependency installation (notably for the ECS Docker image), and formalizes the web server dependency as an optional extra.

Changes:

  • Deleted per-platform requirements.txt files and removed the GitHub workflow that generated them.
  • Added a server optional dependency (aiohttp) and updated uv.lock accordingly.
  • Updated docker/Dockerfile.ecs to install dependencies via uv sync (frozen + pinned) and adjusted docs to no longer reference the removed requirements files.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
uv.lock Adds aiohttp (and transitive deps) plus server extra metadata to keep installs pinned via uv sync.
pyproject.toml Introduces [project.optional-dependencies].server and removes requirements from build source include.
docker/Dockerfile.ecs Switches ECS image dependency installation from requirements+pip to uv sync --frozen --extra server.
docs/CONTRIBUTING.md Removes the requirements install step from contributor setup instructions.
README.md Updates install steps to rely on pyproject.toml/pip install -e . rather than per-OS requirements files.
.github/workflows/make-requirements.yml Removes the workflow that exported and opened PRs for requirements files.
requirements/linux/requirements.txt Deleted (previously autogenerated export).
requirements/macos/requirements.txt Deleted (previously autogenerated export).
requirements/windows/requirements.txt Deleted (previously autogenerated export).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/CONTRIBUTING.md Outdated
Comment thread docs/CONTRIBUTING.md
cd cellpack/
pip install -r requirements/linux/requirements.txt
pip install -e .[dev]
uv sync
Copy link
Copy Markdown
Collaborator Author

@rugeli rugeli Apr 27, 2026

Choose a reason for hiding this comment

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

to address copilot's review comment(slightly out of scope): pip install -e .[dev] has been silently failing on main since the dev lives in [dependency-groups] and pip's [extra] syntax can't read. Switched the contributor setup to use uv sync instead.

@rugeli rugeli requested review from ascibisz and mogres April 27, 2026 23:38
Copy link
Copy Markdown
Collaborator

@mogres mogres left a comment

Choose a reason for hiding this comment

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

Looks great -- thanks!!

@rugeli rugeli merged commit 02bbd72 into main May 6, 2026
10 checks passed
@rugeli rugeli deleted the maint/deprecate-requirements-txt branch May 6, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate requirements.txt files

4 participants