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

rework set_named_arg #14773

Merged
merged 2 commits into from
Jan 27, 2024
Merged

rework set_named_arg #14773

merged 2 commits into from
Jan 27, 2024

Conversation

w-e-w
Copy link
Collaborator

@w-e-w w-e-w commented Jan 27, 2024

change identifying a script from using Scripts class name to Scripts internal name an as not all Script have unique names

raise RuntimeError when there's issue

reason the identifying a script using class name is problematic as currently as there's no convention to using unique names with there Script class name
in base webui alone we have multiple scripts that are all called Scripts
not to mention lots of extensions use the same Script name for the Script

hence the change to using the scrip internal name

using == to match elem_id

  • and as opposed to returning none when there is an issue like script or element id is not found, raise runtime error

  • even though comparing the element ID using in is convenient as it allows you to ignore stuff like the prefix txt2img_ in txt2img_seed but this would cause a potential issue in that depending on the order of your arg
    for example if you have two elements args with IDs of seed and subseed
    if the order of the args is ['seed', 'subseed'] then everything function as normal
    but if the order is ['subseed', 'seed'] when matching for seed it be match with subseed
    hence changed to using ==

Checklist:

change identifying a script from using Scripts class name to Scripts internal name an
as not all Script have unique names

raise RuntimeError when there's issue
@w-e-w w-e-w requested a review from AUTOMATIC1111 as a code owner January 27, 2024 06:25
@w-e-w w-e-w mentioned this pull request Jan 27, 2024
4 tasks
@AUTOMATIC1111
Copy link
Owner

The reason for in instead of == was to be able to use the same name for both img2img and txt2img.

@w-e-w
Copy link
Collaborator Author

w-e-w commented Jan 27, 2024

I forgot to copy the reasoning behind using == for the last PR

tldr is that using in is more flexible but it's also broken

unless you want to add extra logic like if no match if found with == the try matching with in


the tab prefix issue is also in part why I wanted to add update_script_args as even though it's harder to maintain
it is more explicit in its functionality

@w-e-w
Copy link
Collaborator Author

w-e-w commented Jan 27, 2024

we can add an argument fuzzy
set_named_arg(self, args, script_name, arg_elem_id, value, fuzzy=False)
something like if fuzzy use in else ==

@AUTOMATIC1111
Copy link
Owner

Yeah, let's do that.

@w-e-w
Copy link
Collaborator Author

w-e-w commented Jan 27, 2024

done

@w-e-w w-e-w marked this pull request as draft January 27, 2024 11:07
@w-e-w w-e-w force-pushed the rework-set_named_arg branch from 31e4b2e to eae0bb8 Compare January 27, 2024 12:55
@w-e-w w-e-w marked this pull request as ready for review January 27, 2024 12:56
@AUTOMATIC1111 AUTOMATIC1111 merged commit e717eaf into dev Jan 27, 2024
6 checks passed
@AUTOMATIC1111 AUTOMATIC1111 deleted the rework-set_named_arg branch January 27, 2024 17:54
@w-e-w w-e-w mentioned this pull request Feb 17, 2024
@pawel665j pawel665j mentioned this pull request Apr 16, 2024
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.

2 participants