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

Add image generation support with Azure OpenAI #359

Merged
merged 37 commits into from Dec 19, 2023
Merged

Add image generation support with Azure OpenAI #359

merged 37 commits into from Dec 19, 2023

Conversation

jdubois
Copy link
Contributor

@jdubois jdubois commented Dec 15, 2023 •
edited

This uses the same LangChain classes as #298 by @Heezer

  • langchain4j-core/src/main/java/dev/langchain4j/data/image/Image.java
  • langchain4j-core/src/main/java/dev/langchain4j/model/image/ImageModel.java

I copy/pasted those 2 files here as #298 isn't merged yet, and my goal is that we use the same API in the end.

@Heezer Heezer self-requested a review December 16, 2023 08:48
@Heezer
Copy link
Collaborator

Heezer commented Dec 16, 2023

Thanks! There are some refactorings of #298 on the way, the PR is close to be approved. I'll keep you posted!

@jdubois
Copy link
Contributor Author

jdubois commented Dec 16, 2023

Awesome! I got access to Dall-e 3 this morning, so I'll do integration tests.

@jdubois
Copy link
Contributor Author

jdubois commented Dec 16, 2023

And integration tests are passing!

Here's the first ever image created with the integration test AzureOpenAiImageModelIT and it's beautiful:

generated_00

@jdubois jdubois marked this pull request as ready for review December 16, 2023 14:53
Heezer
Heezer requested changes Dec 17, 2023
View reviewed changes
Copy link
Collaborator

@Heezer Heezer left a comment

Choose a reason for hiding this comment

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

Thanks a lot! See my comments.

langchain4j-core/src/main/java/dev/langchain4j/data/image/Image.java Show resolved Hide resolved
langchain4j-core/src/main/java/dev/langchain4j/model/image/ImageModel.java Outdated Show resolved Hide resolved
...ain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/InternalAzureOpenAiHelper.java Outdated Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Outdated Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Show resolved Hide resolved
...ain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/InternalAzureOpenAiHelper.java Outdated Show resolved Hide resolved
@jdubois
Copy link
Contributor Author

jdubois commented Dec 18, 2023

Thanks @Heezer !! This is fixing my 2 biggest issues with this API (not limiting to just 1 image, and being able to get the image content and not just the URL)

@jdubois jdubois requested a review from Heezer December 18, 2023 13:02
langchain4j
langchain4j reviewed Dec 18, 2023
View reviewed changes
Copy link
Owner

@langchain4j langchain4j left a comment

Choose a reason for hiding this comment

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

@jdubois great job, thanks you! Just a few minor things, please check comments

langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Outdated Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Show resolved Hide resolved
langchain4j-azure-open-ai/src/test/java/dev/langchain4j/model/azure/AzureOpenAiChatModelIT.java Outdated Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Outdated Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Outdated Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Show resolved Hide resolved
langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureOpenAiImageModel.java Show resolved Hide resolved
langchain4j-core/src/main/java/dev/langchain4j/data/image/Image.java Outdated Show resolved Hide resolved
@Heezer Heezer mentioned this pull request Dec 18, 2023
Merged
@langchain4j
Copy link
Owner

@jdubois looks good! There is now a conflict due to changes by @Heezer merged to main. Once this is fixed, we can merge 🚀

@jdubois
Fix default deployment names
6997c05
@jdubois
Finalize integration tests
fc8fa21
@jdubois
Update the API usage following code review from @Heezer
77d69b2
@jdubois
Generate only 1 image, and support base64 data
4d5611a
@jdubois
Use default parameters from Azure OpenAI
da89c29
@jdubois
Add documentation for using OpenAI Image generation
2753367
@jdubois
Remove withPersisting and persistTo
3d03312
@jdubois
Add enums in the Builder methods
7882637
@jdubois
Try to fix JavaDoc
ce90c15
@jdubois
AiMessage cannot have a "null" text, so putting "" by default.
2d12aa9
@jdubois
Sync up with the main branch, to use the new Image API.
502f9bd
@jdubois
Merge remote-tracking branch 'origin/azure-openai-image' into azure-o…
b13b357 
…penai-image

# Conflicts:
#	langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/InternalAzureOpenAiHelper.java
#	langchain4j-azure-open-ai/src/test/java/dev/langchain4j/model/azure/AzureOpenAiImageModelIT.java
#	langchain4j-core/src/main/java/dev/langchain4j/data/image/Image.java
#	langchain4j-core/src/main/java/dev/langchain4j/model/image/ImageModel.java
jdubois
jdubois commented Dec 19, 2023
View reviewed changes
Copy link
Contributor Author

@jdubois jdubois left a comment

Choose a reason for hiding this comment

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

All comments from the review should be OK

@jdubois jdubois requested a review from Heezer December 19, 2023 14:12
langchain4j
langchain4j previously approved these changes Dec 19, 2023
View reviewed changes
Copy link
Owner

@langchain4j langchain4j left a comment

Choose a reason for hiding this comment

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

@jdubois just need to remove unused AzureAiMessage and we're good

langchain4j-azure-open-ai/src/main/java/dev/langchain4j/model/azure/AzureAiMessage.java Outdated Show resolved Hide resolved
langchain4j
langchain4j approved these changes Dec 19, 2023
View reviewed changes
Copy link
Owner

@langchain4j langchain4j left a comment

Choose a reason for hiding this comment

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

@jdubois thank you, great job!

@jdubois
Copy link
Contributor Author

jdubois commented Dec 19, 2023

Awesome!! Thanks @langchain4j @Heezer

@langchain4j langchain4j merged commit 76b6f29 into langchain4j:main Dec 19, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glaforge glaforge glaforge left review comments

@langchain4j langchain4j langchain4j approved these changes

@Heezer Heezer Awaiting requested review from Heezer

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@jdubois @Heezer @langchain4j @glaforge