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

VirtualEnv can be created from setup.py files #306

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

nickjalbert
Copy link
Contributor

@nickjalbert nickjalbert commented Mar 7, 2022

This PR allows virtual environments to be created based on the install_requires kwarg found in setup.py files. Also included in this PR:

  • More cache-clearing commands:

    • agentos clear-env-cache
    • agentos clear-repo-cache
    • agentos clear-cache # clears both env and repo cache
  • You can list multiple requirements files (Python package setup.py files or pip-compatible requirements.txt files) in the requirements_path Component spec key separated by a semicolon and all will be installed into the virtual env.

  • Update to rllib_agent and sb3_agent requirements.

  • Minor improvements to the install_requirements.py environment setup script.

  • Remove unneeded requirements from AgentOS setup.py.

Try the following demo in your Python REPL to get a Policy class component out of Ilya's repo:

from agentos import Component, Repo
ilya_repo = Repo.from_github("ikostrikov", "pytorch-a2c-ppo-acktr-gail")
ilya_policy = Component.from_repo( repo=ilya_repo, identifier="Policy==41332b78dfb50321c29bade65f9d244387f68a60", class_name="Policy", file_path="./a2c_ppo_acktr/model.py", requirements_path="./setup.py", instantiate=False)
policy_obj = ilya_policy.get_object()

Fixes #275.

@nickjalbert nickjalbert requested a review from andyk March 9, 2022 13:10
@nickjalbert nickjalbert marked this pull request as ready for review March 9, 2022 13:10
@nickjalbert
Copy link
Contributor Author

Alright @andyk, this should be ready for review!

@nickjalbert nickjalbert changed the title WIP: setup.py VirtualEnv can be created from setup.py files Mar 9, 2022
Copy link
Contributor

@andyk andyk left a comment

Choose a reason for hiding this comment

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

LGTM! Just 3 nits/typos in docstrings and one possible suggestion for an assert statement.

"""
This command clears all virtual environments that have been cached by
AgentOS in your local file system. All the virtual environments can be
automatically recreated when re-running a Component ``requirements_path``
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: missing "with" in "re-running a Component with `requirements_path` specified."

def clear_repo_cache(assume_yes):
"""
This command clears all git repos that have been cached by AgentOS on your
local file system. These repos will be recreated if as you run Components
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: extra "if" in "These repos will be recreated if as you run..."

for req_path in c.requirements_path.split(";"):
full_req_path = self.repo.get_local_file_path(
c.identifier.version, req_path
).absolute()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to assert that the requirements_file exists here?

@andyk
Copy link
Contributor

andyk commented Mar 12, 2022

@nick I think this might fix #297?

@nickjalbert
Copy link
Contributor Author

nickjalbert commented Mar 14, 2022

I think this might fix #297?

I don't think this goes quite as far as I'm hoping to in #297, I think we should keep that open for now...

Actually, just looked at virtual_env.py again, and I'm happy closing #297 for now!

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.

Support requirements declared via setup.py
2 participants