Skip to content
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

Use pyproject instead of reqs.txt #6

Merged
merged 1 commit into from
Apr 13, 2024
Merged

Use pyproject instead of reqs.txt #6

merged 1 commit into from
Apr 13, 2024

Conversation

granawkins
Copy link
Member

update requirements to use pyproject only and use latest spiceai minor version.

@@ -15,7 +15,7 @@ dependencies = [
"fastapi==0.109.2",
"Jinja2==3.1.3",
"networkx==3.2.1",
"spiceai==0.1.11",
"spiceai~=0.1.0",
Copy link

Choose a reason for hiding this comment

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

Using the ~= operator for spiceai dependency will install the latest minor version, but it might introduce breaking changes if not properly managed. Consider specifying a more precise version if stability is a priority.

@@ -24,8 +24,8 @@ jobs:
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
pip install -e .
Copy link

Choose a reason for hiding this comment

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

It's good practice to specify the version of actions used in workflows to avoid unexpected changes. For example, consider using actions/checkout@v2 and actions/setup-python@v2.

Copy link

mentatbot bot commented Apr 13, 2024

MENTAT CODE REVIEW IN ACTIVE DEVELOPMENT. Only in use on mentat and internal repos.
Please Reply with feedback.

The pull request successfully transitions dependency management to pyproject.toml and removes requirements.txt, aligning with Python packaging best practices. However, consider specifying more precise versions for GitHub Actions to ensure workflow stability. Also, be cautious with the use of the ~= operator for dependencies like spiceai to avoid potential breaking changes with new minor versions.

@granawkins granawkins merged commit b5eaf1b into main Apr 13, 2024
3 checks passed
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.

1 participant