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

Added SD Lora Tagger to extensions #285

Draft
wants to merge 1 commit into
base: extensions
Choose a base branch
from

Conversation

corbin-hayden13
Copy link

@corbin-hayden13 corbin-hayden13 commented Feb 27, 2024

Info

Repo URL here.
This extension is being actively maintained and developed.
I wasn't sure whether or not to tag the extension with "online" because there's a feature to make api requests to Civitai but this is triggered by a button click and not part of the main function of the extension, but I tagged it anyways with "online" to be safe.
Thanks for your consideration!

Checklist:

  • I have read the Readme.md
  • The description is written in English.
  • The index.json and extension_template.json have not been modified.
  • The entry is placed in the extensions directory with the .json file extension.

@w-e-w
Copy link
Collaborator

w-e-w commented Mar 6, 2024

sorry I am on vacation so I'm a bit solw on doing extension reviews

I did a quick test a found that your extension is built for webui 1.7 and is compatible with the current master version 1.8
since 1.8 is out it need to work with 1.8 before it can be put on the index

note at time that you submitted the extension webui was in 1.8-RC and about 4 days ago version 1.8 was released


other minor issue
you seem to have warning that is specifically for vlads sd.next
like The LyCORIS extension is not installed in the "extensions-builtin\ file path, cannot overwrite.
it's good that you made compatibility with sd.next but the issue is that this will cause confusion with base webui users
see https://github.com/KohakuBlueleaf/a1111-sd-webui-lycoris?tab=readme-ov-file#a1111-sd-webui-lycoris-sd-webui-forge-sd-webui150


personal opinion
I quite like this extension
I had a similar idea but didn't have time to implement it myself

@w-e-w
Copy link
Collaborator

w-e-w commented Mar 7, 2024

another issue

because the namespace shared by the base webui and all other extentions and the you should avoid commn names for python scripts
in your case you should not use the file name globals.py because if someone decidesto use the same name, things might break

I my opinion the current best pretices is to put lib scripts under a diffrent dir so you don't have to worrarbout name collisions
example from one of my extention https://github.com/w-e-w/sd-webui-hires-fix-tweaks
I have only one file in scripts sd-webui-hires-fix-tweaks.py all other files are imported from hires_fix_tweaks

but this is for larger more complex projects
for simple project you can get away with using use unique name
examp you can rename it to sd_lora_tagger_globals.py

@w-e-w
Copy link
Collaborator

w-e-w commented Mar 10, 2024

so this is kind of awkward
couple of hours ago someone made a PR to webui

so yeah... basically what you're extension provide is now or will be a built-in feature (webui dev branch)

this is just a heads up so that you can make your decision on whether or not to continue development

@w-e-w
Copy link
Collaborator

w-e-w commented Mar 10, 2024

saw ther was a 1.8 fix commit

but it still doens't seem t be fixed

Traceback (most recent call last):
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\routes.py", line 488, in run_predict
    output = await app.get_blocks().process_api(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\blocks.py", line 1431, in process_api
    result = await self.call_function(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\blocks.py", line 1103, in call_function
    prediction = await anyio.to_thread.run_sync(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\anyio\to_thread.py", line 33, in run_sync
    return await get_asynclib().run_sync_in_worker_thread(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\anyio\_backends\_asyncio.py", line 877, in run_sync_in_worker_thread
    return await future
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\anyio\_backends\_asyncio.py", line 807, in run
    result = context.run(func, *args)
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\utils.py", line 707, in wrapper
    response = f(*args, **kwargs)
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 703, in pages_html
    create_html()
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 699, in create_html
    ui.pages_contents = [pg.create_html(ui.tabname) for pg in ui.stored_extra_pages]
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 699, in <listcomp>
    ui.pages_contents = [pg.create_html(ui.tabname) for pg in ui.stored_extra_pages]
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 547, in create_html
    "tree_html": self.create_tree_view_html(tabname),
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 472, in create_tree_view_html
    item_html = self.create_tree_dir_item_html(tabname, k, _build_tree(v))
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 465, in _build_tree
    _dir_li.append(self.create_tree_dir_item_html(tabname, k, _build_tree(v)))
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 463, in _build_tree
    _file_li.append(self.create_tree_file_item_html(tabname, k, v.item))
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 409, in create_tree_file_item_html
    "data_hash": item["shorthash"],
KeyError: 'shorthash'
Traceback (most recent call last):
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\routes.py", line 488, in run_predict
    output = await app.get_blocks().process_api(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\blocks.py", line 1431, in process_api
    result = await self.call_function(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\blocks.py", line 1103, in call_function
    prediction = await anyio.to_thread.run_sync(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\anyio\to_thread.py", line 33, in run_sync
    return await get_asynclib().run_sync_in_worker_thread(
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\anyio\_backends\_asyncio.py", line 877, in run_sync_in_worker_thread
    return await future
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\anyio\_backends\_asyncio.py", line 807, in run
    result = context.run(func, *args)
  File "B:\GitHub\stable-diffusion-webui\venv\lib\site-packages\gradio\utils.py", line 707, in wrapper
    response = f(*args, **kwargs)
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 703, in pages_html
    create_html()
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 699, in create_html
    ui.pages_contents = [pg.create_html(ui.tabname) for pg in ui.stored_extra_pages]
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 699, in <listcomp>
    ui.pages_contents = [pg.create_html(ui.tabname) for pg in ui.stored_extra_pages]
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 547, in create_html
    "tree_html": self.create_tree_view_html(tabname),
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 472, in create_tree_view_html
    item_html = self.create_tree_dir_item_html(tabname, k, _build_tree(v))
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 465, in _build_tree
    _dir_li.append(self.create_tree_dir_item_html(tabname, k, _build_tree(v)))
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 463, in _build_tree
    _file_li.append(self.create_tree_file_item_html(tabname, k, v.item))
  File "B:\GitHub\stable-diffusion-webui\modules\ui_extra_networks.py", line 409, in create_tree_file_item_html
    "data_hash": item["shorthash"],
KeyError: 'shorthash'

tested with corbin-hayden13/SD-Lora-Tagger@0055952 on webui version: v1.8.0


set this PR as draft for now, please leave a message when it's fixed

@w-e-w w-e-w marked this pull request as draft March 10, 2024 13:59
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