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.

Sign up for GitHub

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

Jump to bottom

Extraction Engine Improvements #1097

Merged
merged 14 commits into from
Oct 4, 2024
Merged

Extraction Engine Improvements #1097

merged 14 commits into from
Oct 4, 2024

Conversation

collindutter
Copy link
Member

@collindutter collindutter commented Aug 21, 2024 •
edited
Loading

Describe your changes

Changed

  • BREAKING: Split BaseExtractionEngine.extract into extract and extract_artifacts for consistency with BaseSummaryEngine.
  • BREAKING: BaseExtractionEngine no longer catches exceptions and returns ErrorArtifacts.
  • BREAKING: JsonExtractionEngine.template_schema is now required.
  • BREAKING: CsvExtractionEngine.column_names is now required.
  • JsonExtractionEngine.extract_artifacts now returns a ListArtifact[JsonArtifact].
  • CsvExtractionEngine.extract_artifacts now returns a ListArtifact[CsvRowArtifact].

Issue ticket number and link

NA

📚 Documentation preview 📚: https://griptape--1097.org.readthedocs.build//1097/

@collindutter collindutter mentioned this pull request Aug 21, 2024
Merged
1 task
collindutter
collindutter commented Aug 21, 2024
View reviewed changes
griptape/artifacts/json_artifact.py Outdated
Comment on lines 13 to 14
Json = Union[dict[str, "Json"], list["Json"], str, int, float, bool, None]

Copy link
Member Author

@collindutter collindutter Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Allows us to access via JsonArtifact.Json which may be useful in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, could you provide a hypothetical example?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the near future we may offer some sort of structured output functionality that would allow users to pass in a JSON schema they'd like the LLM to output their answer in.

The type could be something like: output_format: JsonArtifact.Json

@collindutter collindutter force-pushed the feature/json-extraction branch 2 times, most recently from 317ec50 to 0c788ef Compare August 21, 2024 22:33
@collindutter collindutter force-pushed the feature/json-extraction branch from 0c788ef to 004e36a Compare August 21, 2024 22:33
@collindutter collindutter marked this pull request as ready for review August 21, 2024 22:34
@collindutter collindutter force-pushed the feature/list-artifact branch from 1c7f97d to a7f10d4 Compare August 22, 2024 16:31
dylanholmes
dylanholmes reviewed Aug 22, 2024
View reviewed changes
Copy link
Contributor

@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.

Nice work!

CHANGELOG.md Show resolved Hide resolved
docs/griptape-framework/engines/extraction-engines.md Outdated Show resolved Hide resolved
griptape/artifacts/json_artifact.py Outdated
Comment on lines 13 to 14
Json = Union[dict[str, "Json"], list["Json"], str, int, float, bool, None]

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, could you provide a hypothetical example?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
dylanholmes
dylanholmes requested changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@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.

Just a couple tiny things

docs/griptape-framework/engines/extraction-engines.md Outdated Show resolved Hide resolved
docs/griptape-framework/engines/extraction-engines.md Outdated Show resolved Hide resolved
dylanholmes
dylanholmes requested changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@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.

Just one typo!

docs/griptape-framework/engines/extraction-engines.md Outdated Show resolved Hide resolved
@collindutter
Copy link
Member Author

Gonna hold off on merging this until some of the artifact changes are settled.

@collindutter collindutter changed the base branch from feature/list-artifact to dev September 14, 2024 00:29
@codecov Codecov
Copy link

codecov bot commented Sep 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@collindutter collindutter force-pushed the feature/json-extraction branch from fa52568 to 89a800a Compare October 3, 2024 18:35
@collindutter
Copy link
Member Author

@vachillo bump on this since it's an oldie

@collindutter collindutter merged commit 79f976b into dev Oct 4, 2024
14 of 15 checks passed
@collindutter collindutter deleted the feature/json-extraction branch October 4, 2024 21:28
@collindutter collindutter mentioned this pull request Oct 4, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylanholmes dylanholmes dylanholmes approved these changes

@vachillo vachillo Awaiting requested review from vachillo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@collindutter @dylanholmes