Conversation
There was a problem hiding this comment.
Pull request overview
Migrates Python dependency management from requirements.txt to pyproject.toml, and updates CI/docs to install the package (and dev extras) via pip from the project metadata.
Changes:
- Remove
requirements.txtand rely on[project.dependencies]/[project.optional-dependencies]inpyproject.toml. - Update GitHub Actions workflows to
pip install -e ./pip install -e .[dev]instead ofpip install -r requirements.txt. - Add a brief “building from source” install note to the README.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | Removed legacy dependency list in favor of pyproject.toml. |
| pyproject.toml | Adds pydantic to core deps; moves typing stubs into dev extra. |
| README.md | Documents source install command. |
| .github/workflows/pr-scala.yml | CI now installs the Python package from the repo root. |
| .github/workflows/pr-python.yml | CI now installs package/dev extra instead of requirements.txt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pyproject.toml
Outdated
| "typing_extensions", | ||
| "types-Deprecated==1.2.9.20240311", | ||
| "types-protobuf" | ||
| ] | ||
|
|
There was a problem hiding this comment.
typing_extensions is already a core dependency (typing_extensions >= 4.4.0), so listing it again in the dev extra is redundant. Removing the duplicate entry will reduce confusion and keep dependency definitions single-sourced.
| "typing_extensions", | |
| "types-Deprecated==1.2.9.20240311", | |
| "types-protobuf" | |
| ] | |
| "types-Deprecated==1.2.9.20240311", | |
| "types-protobuf" | |
| ] |
.github/workflows/pr-python.yml
Outdated
| run: | | ||
| pip install -r requirements.txt | ||
| pip install -e .[dev] | ||
| pip install black |
There was a problem hiding this comment.
The workflow installs .[dev] (which already includes black) and then installs black again. This is redundant and can introduce nondeterminism if the extra and the subsequent install resolve to different versions; prefer installing black in one place (either via the dev extra, or remove it from the extra and install it explicitly here).
| pip install black |
README.md
Outdated
|
|
||
| **Setup tl;dr**: install the Python package from pip: `pip install edg`, and optionally run the [IDE plugin with block diagram visualizer](setup.md#ide-setup). | ||
|
|
||
| Building from source: `pip install .` at the repository root. |
There was a problem hiding this comment.
There’s an extra space after the colon and trailing whitespace at the end of this line. Cleaning this up avoids markdown lint noise and keeps formatting consistent.
| Building from source: `pip install .` at the repository root. | |
| Building from source: `pip install .` at the repository root. |
pyproject.toml
Outdated
| @@ -21,12 +21,16 @@ dependencies = [ | |||
| "sexpdata==0.0.3", | |||
| "Deprecated", | |||
There was a problem hiding this comment.
Since requirements.txt is being removed, the effective constraint for Deprecated becomes unpinned ("Deprecated"). Previously it was pinned to 1.2.14 in requirements.txt; consider pinning it here as well to avoid accidental upgrades breaking runtime behavior/CI reproducibility.
| "Deprecated", | |
| "Deprecated==1.2.14", |
No description provided.