-
Notifications
You must be signed in to change notification settings - Fork 267
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
Add API Endpoints [POC] #611
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea! The PR needs some cleaning up, though; see within, please :)
scripts/api.py
Outdated
@@ -0,0 +1,46 @@ | |||
import gradio as gr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As much as possible should be in the sd_dynamic_prompts
package, please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved.
self, | ||
p, | ||
is_enabled: bool, | ||
is_combinatorial: bool, | ||
combinatorial_batches: int, | ||
is_magic_prompt: bool, | ||
is_feeling_lucky: bool, | ||
is_attention_grabber: bool, | ||
min_attention: float, | ||
max_attention: float, | ||
magic_prompt_length: int, | ||
magic_temp_value: float, | ||
use_fixed_seed: bool, | ||
unlink_seed_from_prompt: bool, | ||
disable_negative_prompt: bool, | ||
enable_jinja_templates: bool, | ||
no_image_generation: bool, | ||
max_generations: int, | ||
magic_model: str | None, | ||
magic_blocklist_regex: str | None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the spurious formatting changes here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
original_negative_prompt, | ||
) | ||
|
||
def generate_prompts( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just generating prompts shouldn't require the full p
object, I don't think..?
Further, maybe this should be made a free function, and it should probably be kwarg-only ((*, original_prompt...)
) because otherwise it's super easy to pass arguments in the wrong order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p
is used for its additional seed settings. see around dynamic_prompting.py.py#558
and around dynamic_prompting.py.py#572
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, but those probably ought to be moved on from this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing an easy way to move them out, any suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, GeneratorBuilder().set_context(p)
is a bit problematic, but that's only used for Jinja context – the p-to-context code in create_jinja_generator
should be refactored out. (I think Jinja over the API would fail right now since p.sd_model
will not get set.)
With that done, I the p.all_seeds, p.all_subseeds = get_seeds(...)
manipulation could be done outside the prompt-generation function, couldn't it?
@akx Hey, I am still looking for review feedback. I think I addressed everything except removing |
@ArrowM ICYMI, I replied to the review comment :) |
@akx Hey, sorry for the really delayed response. I don't plan to dig into any of the p context code to refactor this further. I'll leave this to you to either run with it or close the PR. Thanks for your work on the extension! <3 |
I would like to add endpoints for generating prompts. I found "Don't generate images" to be clunky. I created a basic one that covers my use case, but it could easily be expanded to cover the other ~15 params. The biggest change here is splitting some of the
process()
code into a separate method for generatingall_prompts
andnegative_prompts
New endpoint can be found at http://127.0.0.1:7860/docs#/default/evaluate_dynamicprompts_evaluate_post