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

Overhaul repo to support plugins #3

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

collindutter
Copy link
Member

Overhauls the repo to support more than just Tools. Would like feedback on:

  1. Does the term "plugin" make sense? "Extension"?
  2. Do the README instructions make sense? Would you know how to create a Prompt Driver?

@collindutter collindutter force-pushed the refactor/plugin-overhaul branch 2 times, most recently from 195e966 to 4a09284 Compare November 18, 2024 17:32
@collindutter
Copy link
Member Author

3.12 failing is expected, it's already fixed on griptape@dev

@collindutter collindutter force-pushed the refactor/plugin-overhaul branch from 4a09284 to 6377f28 Compare November 18, 2024 17:34
Copy link

@dylanholmes dylanholmes left a comment

Choose a reason for hiding this comment

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

I'm fine with "plugin".

I think it's pretty straight forward on how you'd add a prompt driver, but you could provide links to griptape maintained plugin repos for more in depth examples (once we have them).

source .venv/bin/activate
source $VENV

Choose a reason for hiding this comment

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

TIL

Comment on lines -10 to +14
test:
test-ubuntu:

Choose a reason for hiding this comment

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

Windows too?

@collindutter collindutter force-pushed the refactor/plugin-overhaul branch 3 times, most recently from 07f44dc to 0c72d3a Compare November 18, 2024 18:57
Copy link
Member

@SavagePencil SavagePencil left a comment

Choose a reason for hiding this comment

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

We've said what but not why. Small inserts can fix that.

README.md Outdated

A Github template repository for creating Griptape tools.
A Github template repository for creating Griptape plugins.
Copy link
Member

Choose a reason for hiding this comment

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

Suggest "extensions" as it implies extending functionality and capabilities. Opinion, tho.

Copy link

Choose a reason for hiding this comment

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

I don't have a strong opinion either way.. plugins or extensions both work for me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I like extension more. Plugin, to me, suggests some sort of modification of core behavior. This template is for providing new, extended functionality.

README.md Outdated
Comment on lines 13 to 11
Via github web page:

Click on `Use this template`

![](https://docs.github.com/assets/cb-36544/images/help/repository/use-this-template-button.png)
Copy link
Member

Choose a reason for hiding this comment

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

Make this one first. Everyone here will know that one, not everyone will be as familiar with the CLI

Copy link

Choose a reason for hiding this comment

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

agreed

README.md Outdated

## What is a Griptape Plugin?

A Griptape Plugin is nothing more than a python class that implements one of Griptape's interfaces.
Copy link
Member

Choose a reason for hiding this comment

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

Start with what it affords, e.g., "A Griptape [extension/plugin] allows you to make your own functionality for your Griptape agents..." Since you are already using the 2nd person elsewhere in this document, the second sentence can be "You create one as a Python class that implements a Griptape interface."

You've told me what one is but now why I would want to use one. Give some concrete examples.

Copy link

Choose a reason for hiding this comment

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

I understand the reason behind saying "nothing more than" because it makes it sound simple and easy.. but it ends up sounding not very impressive. Maybe just simplify?

also, not sure what "implements one of Griptape's interfaces" means. When I hear "interfaces" I think "UI".

README.md Outdated
Comment on lines 58 to 92
### Installing Dependencies

```bash
make install
```

### Running Tests

```bash
make test
```

### Running Checks

```bash
make check
```

### Running Formatter

```bash
make format
```

Copy link
Member

Choose a reason for hiding this comment

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

Should these be at #### (four indents) as they appear to be subsets of make setup

```bash
make setup
```

Copy link
Member

Choose a reason for hiding this comment

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

Summarize what that results in (e.g., "This will install all dependencies, run tests to validate no regressions, run checks (what is that?), and run the formatter (why isn't it formatted already?).

README.md Outdated

### Running Example

1. Set appropriate environment variables. The example requires an `OPENAI_API_KEY` environment variable to be set.
Copy link
Member

Choose a reason for hiding this comment

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

can you do a 1a. that says "each example will require its own environment variables. See the XXX in each example to understand which environment variables are required to run them."

```

### Installing In Other Projects

Copy link
Member

Choose a reason for hiding this comment

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

Talk about why and when you would want to do this, possibly with an example.

README.md Outdated
2. Run the example:

```bash
poetry run python examples/example.py
Copy link

Choose a reason for hiding this comment

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

This assumes the user is using poetry - should we make that assumption? do we want to include pip instructions as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

This repo is heavily dependent on poetry. I think it's fine as long we provide some justification for it. I'll include some more context about what poetry is and why we're using it.


The template repository is structured as follows:

├── griptape
Copy link

Choose a reason for hiding this comment

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

This wasn't showing as a clean hierarchy in the preview for me - do you want to wrap it in ```?

Copy link

@shhlife shhlife left a comment

Choose a reason for hiding this comment

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

made a couple of notes

```

#### Running Example

Copy link
Member

Choose a reason for hiding this comment

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

  1. We gave you an example you can run
  2. Here's what it does in plain English
  3. Here's what role the plugin/extension plays in that experience
  4. Here's where to look for when you want to learn how the plugin/extension was built and hooked up

Copy link
Member

@SavagePencil SavagePencil left a comment

Choose a reason for hiding this comment

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

get ahead...ahead of that dread!

README.md Outdated

## What is a Griptape Extension?

Griptape Extensions can add new functionality to the [griptape framework](https://github.com/griptape-ai/griptape), such as new Tools, Drivers, Tasks, or Structures.
Copy link
Member

Choose a reason for hiding this comment

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

Be clear what that affords. "With Griptape Extensions, your Agents can now do X, Y, Z."

Also capitalize Griptape framework

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you have in mind for X, Y, Z? Extensions don't do anything concrete on their own, they simply enable the user to develop functionality like new Tools, Drivers, Tasks, or Structures.

Copy link
Member

Choose a reason for hiding this comment

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

right, but that may not be obvious what that affords. "With extensions, your Griptape Agents can leverage your team's APIs, perform custom business logic, and access proprietary data sources, all as part of their reasoning."

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a sentence.

README.md Outdated

### Installing in Other Projects

Extensions are designed to be shared and installed in projects that can benefit from them.
Copy link
Member

Choose a reason for hiding this comment

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

How about Extensions are designed to be shared. Extensions are made to easily install (include? import?) into existing Python projects.

README.md Outdated

Extensions are designed to be shared and installed in projects that can benefit from them.

Although publishing to [PyPI](https://pypi.org/) is an option, it is beyond the scope of this template. Instead, extensions can be installed directly from the repository.
Copy link
Member

Choose a reason for hiding this comment

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

I'd remove the PyPi comment, because it gives the reader a sense of dread (n.b.: I am always sensing dread). Be optimistic: The easiest way to include your extension into an existing project is to install directly from the repository, like so:

...

later:
Advanced customers may seek to publish their extensions to PyPi. Those instructions are beyond the scope of this README.

@collindutter collindutter force-pushed the refactor/plugin-overhaul branch from 8728935 to 32428ca Compare November 19, 2024 16:55
@collindutter collindutter merged commit 02138f1 into main Nov 19, 2024
8 of 9 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.

4 participants