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

Drop requirements.txt in favor of pyproject.toml and add autogpt entrypoint #1955

Closed
wants to merge 1 commit into from

Conversation

jayceslesar
Copy link
Contributor

@jayceslesar jayceslesar commented Apr 16, 2023

Background

The idea here is to drop requirements.txt in favor of pyproject.toml which is the preferred way to handle projects as scale such as this. This opens up the door to a more unified CI as time passes. This also allows the ability to split out dev dependencies and application dependencies in the future if needed. This also adds an autogpt entrypoint so users no longer have to run python3 -m autogpt for clarity.

Changes

Change documentation around installing to just pip install . and pip install -e . if needed. No setup should change other than that and relevant docker commands were changed to match this.

Documentation

The README.md was changed in order to show that the requirements.txt is not needed anymore and one can simply pip install . and also add an entrypoint so users of the application level code can just run autogpt with the same CLI exposed behind the previous way of invoking the application.

Test Plan

Testing shouldnt really need to be done

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts. # N/A
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

This doesn't change any functional code only how the code/application is installed and invoked.

Comment on lines 34 to +35
// Use 'postCreateCommand' to run commands after the container is created.
// "postCreateCommand": "pip3 install --user -r requirements.txt",
// "postCreateCommand": "pip3 install --user .",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This probably doesnt work

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Apr 17, 2023
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

1 similar comment
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@Pwuts Pwuts added the bootstrapping Start-up process of the app label Apr 20, 2023
@Pwuts Pwuts self-assigned this Apr 20, 2023
@Pwuts
Copy link
Member

Pwuts commented Apr 20, 2023

@jayceslesar can you resolve the merge conflicts and make sure it correctly handles the conditional dependencies?
https://github.com/Significant-Gravitas/Auto-GPT/blob/2f053fe9db74a59001715a64791f1c983fbd46a0/requirements.txt#L36-L49

@Pwuts Pwuts mentioned this pull request Apr 21, 2023
5 tasks
@Pwuts Pwuts changed the base branch from stable to master April 22, 2023 10:36
@edcohen08
Copy link
Contributor

If we're going this direction (which i like a lot) why not just switch to poetry completely and use a poetry.lock for dependency management? way prefer how poetry handles installs and virtual environments

@jayceslesar
Copy link
Contributor Author

@Pwuts is this still something you guys want to implement? I like the poetry suggestion mentioned above...getting the dependencies not to exist in docker may be tricky...excluding dependencies in extras is not really feasible as they are "extras"

@Pwuts
Copy link
Member

Pwuts commented Apr 27, 2023

I think so, yes. It's just that we also have more pressing todos so we haven't gotten to discussing this yet.

@p-i-
Copy link
Contributor

p-i- commented May 5, 2023

This is a mass message from the AutoGPT core team.
Our apologies for the ongoing delay in processing PRs.
This is because we are re-architecting the AutoGPT core!

For more details (and for infor on joining our Discord), please refer to:
https://github.com/Significant-Gravitas/Auto-GPT/wiki/Architecting

@ntindle
Copy link
Member

ntindle commented May 20, 2023

I know this is a bit old but if you're interested in getting this merged, so am I

@ntindle ntindle self-assigned this May 20, 2023
@Pwuts
Copy link
Member

Pwuts commented Jun 7, 2023

If we want to get this merged, we'll probably have to do it ourselves

@ntindle
Copy link
Member

ntindle commented Jun 10, 2023

Fair, at this point, its probably worth just reopening from scratch just to avoid conflicts

@ntindle ntindle removed their assignment Jun 10, 2023
@Pwuts Pwuts mentioned this pull request Jun 15, 2023
5 tasks
@collijk collijk mentioned this pull request Jul 17, 2023
26 tasks
@Pwuts
Copy link
Member

Pwuts commented Sep 14, 2023

Duplicate of #1102

@Pwuts Pwuts marked this as a duplicate of #1102 Sep 14, 2023
@Pwuts Pwuts closed this Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bootstrapping Start-up process of the app conflicts Automatically applied to PRs with merge conflicts
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants