Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the project from traditional setuptools with setup.py to modern Python packaging using uv as the primary package manager and build tool. The migration simplifies dependency management, build processes, and CI/CD workflows while maintaining backward compatibility.
Changes:
- Replaced
setup.pywithpyproject.tomlusinghatchlingas the build backend - Migrated all GitHub Actions workflows to use
uvfor dependency installation, testing, and publishing - Updated version management to use
uv versioncommand and dynamic version loading viaimportlib.metadata - Enhanced
algorithm_loader.pyto preferuv pipoverpipwhen available
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Removed legacy setuptools-based configuration file |
| pyproject.toml | Added modern PEP 621 compliant project configuration with uv-specific settings |
| neuracore/init.py | Updated to use dynamic version loading from package metadata |
| neuracore/ml/utils/algorithm_loader.py | Enhanced to prefer uv pip when available with graceful pip fallback |
| .bumpversion.cfg | Removed in favor of uv's built-in version management |
| .github/workflows/*.yaml | Updated all CI/CD workflows to use astral-sh/setup-uv@v5 and uv commands |
| README.md | Added uv installation instructions and updated development/testing commands |
| cSpell.json | Added pyproject.toml and uv.lock to ignore list |
| neuracore-dictionary.txt | Added "venv" to custom dictionary |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [dependency-groups] | ||
| dev = [ | ||
| "pytest>=6.2.5", | ||
| "pytest-cov>=2.12.1", | ||
| "pytest-asyncio>=0.15.1", | ||
| "pytest-xdist", | ||
| "requests-mock>=1.9.3", | ||
| "pre-commit", | ||
| ] |
There was a problem hiding this comment.
The dev dependencies have been moved from [project.optional-dependencies] to [dependency-groups]. This is a breaking change for users who install dev dependencies using pip install .[dev] - they should now use uv sync instead. Consider documenting this migration path or maintaining backward compatibility by keeping dev in [project.optional-dependencies] as well.
| "psutil", | ||
| "typer>=0.20.0", | ||
| "neuracore-types>=4.3.0,<5.0.0", | ||
| "pip", # For backwards compatibility |
There was a problem hiding this comment.
Adding 'pip' as a dependency is unusual and potentially problematic. Since pip is typically bundled with Python and uv doesn't require pip to be installed, this dependency may cause conflicts or confusion. Consider removing this dependency or clarifying why it's needed for backward compatibility.
| "pip", # For backwards compatibility |
| # Use uv version to bump | ||
| uv version --bump $BUMP_TYPE | ||
| NEW_VERSION=$(uv version) | ||
|
|
There was a problem hiding this comment.
The command uv version --bump may not be correct syntax. Based on uv's CLI, version bumping might require a different command format. Verify that uv version --bump major|minor|patch is the correct syntax, as the documentation typically shows commands like uv version to display the current version. This could cause the version bump workflow to fail.
| # Use uv version to bump | |
| uv version --bump $BUMP_TYPE | |
| NEW_VERSION=$(uv version) | |
| # Bump version in pyproject.toml using Python | |
| NEW_VERSION=$(BUMP_TYPE="$BUMP_TYPE" python - << 'PY' | |
| import os | |
| import pathlib | |
| import re | |
| import sys | |
| bump_type = os.environ.get("BUMP_TYPE", "").strip().lower() | |
| if bump_type not in {"major", "minor", "patch"}: | |
| print(f"Invalid BUMP_TYPE: {bump_type!r}", file=sys.stderr) | |
| sys.exit(1) | |
| path = pathlib.Path("pyproject.toml") | |
| text = path.read_text(encoding="utf-8") | |
| # Extract [project] section | |
| project_match = re.search(r'(?ms)^\\[project\\]\\s*(.+?)(?=^\\[|\\Z)', text) | |
| if not project_match: | |
| print("Could not find [project] section in pyproject.toml", file=sys.stderr) | |
| sys.exit(1) | |
| project_block = project_match.group(1) | |
| # Find version line like: version = "X.Y.Z" | |
| version_match = re.search(r'(?m)^version\\s*=\\s*"(?P<maj>\\d+)\\.(?P<min>\\d+)\\.(?P<pat>\\d+)"', project_block) | |
| if not version_match: | |
| print("Could not find project version in [project] section", file=sys.stderr) | |
| sys.exit(1) | |
| major = int(version_match.group("maj")) | |
| minor = int(version_match.group("min")) | |
| patch = int(version_match.group("pat")) | |
| if bump_type == "major": | |
| major, minor, patch = major + 1, 0, 0 | |
| elif bump_type == "minor": | |
| minor, patch = minor + 1, 0 | |
| else: # "patch" | |
| patch += 1 | |
| new_version = f"{major}.{minor}.{patch}" | |
| # Replace version line within the project block | |
| new_project_block = re.sub( | |
| r'(?m)^version\\s*=\\s*".*"', | |
| f'version = "{new_version}"', | |
| project_block, | |
| count=1, | |
| ) | |
| # Reconstruct full file | |
| new_text = text[:project_match.start(1)] + new_project_block + text[project_match.end(1):] | |
| path.write_text(new_text, encoding="utf-8") | |
| # Print the new version so the shell can capture it | |
| sys.stdout.write(new_version) | |
| PY | |
| ) |
| from .core.exceptions import * # noqa: F403 | ||
|
|
||
| __version__ = "7.9.1" | ||
| __version__ = version("neuracore") |
There was a problem hiding this comment.
Using importlib.metadata.version() to dynamically load the version will fail when the package is not installed (e.g., during development in editable mode without installation). This breaks the workflow where developers clone the repo and import neuracore directly. Consider keeping a fallback to read from pyproject.toml or maintaining a static version string for development scenarios.
| # Try uv first if available, then fall back to pip | ||
| uv_path = shutil.which("uv") | ||
| if uv_path: | ||
| cmd = [uv_path, "pip", "install", "-r", str(req_file)] |
There was a problem hiding this comment.
When using uv pip install, you should consider passing the -I flag (like the pip fallback) to ignore installed packages and ensure fresh installations. The inconsistency between uv and pip command flags could lead to different installation behaviors.
| cmd = [uv_path, "pip", "install", "-r", str(req_file)] | |
| cmd = [uv_path, "pip", "install", "-I", "-r", str(req_file)] |
|
|
||
| ```bash | ||
| pip install neuracore | ||
| uv add neuracore |
There was a problem hiding this comment.
The command uv add neuracore is for adding dependencies to an existing uv-managed project. For users who want to install neuracore as a standalone package (not as a dependency of another project), they should use uv pip install neuracore or continue using pip. Consider clarifying the installation instructions to distinguish between these use cases.
Features
uvwhere-ever possible.