hiddenswitch / ComfyUI

A powerful and modular stable diffusion GUI with a graph/nodes interface.
GNU General Public License v3.0
30 stars 10 forks source link

Allow iteration over folder paths #4

Closed cjonesuk closed 4 months ago

cjonesuk commented 5 months ago

When installing the ComfyUI IP Adapter Plus custom nodes, comfyui fails to import due to being unable iterate over the folder paths. This PR overrides the iterator.

Error

  File "E:\dev\sd\sd-experiment\custom_nodes\ComfyUI_IPAdapter_plus\IPAdapterPlus.py", line 23, in <module>
    if "ipadapter" not in folder_paths.folder_names_and_paths:
  File "E:\dev\sd\sd-experiment\.venv\lib\site-packages\comfy\cmd\folder_paths.py", line 47, in __getitem__
    raise RuntimeError("expected folder path")
RuntimeError: expected folder path

Cannot import E:\dev\sd\sd-experiment\custom_nodes\ComfyUI_IPAdapter_plus module for custom nodes: expected folder path

IPAdapterPlus.py file where the error occurs:

# set the models directory backward compatible
GLOBAL_MODELS_DIR = os.path.join(folder_paths.models_dir, "ipadapter")
MODELS_DIR = GLOBAL_MODELS_DIR if os.path.isdir(GLOBAL_MODELS_DIR) else os.path.join(os.path.dirname(os.path.realpath(__file__)), "models")
if "ipadapter" not in folder_paths.folder_names_and_paths:
    current_paths = [MODELS_DIR]
else:
    current_paths, _ = folder_paths.folder_names_and_paths["ipadapter"]
doctorpangloss commented 4 months ago

thanks for the ticket. I have long ago integrated my own, single file copy of ipadapter which you can find here: https://gist.github.com/doctorpangloss/7646f65c6ecba6c359291ccd081bfea1 and it is compatible with this fork, using some of its nice new features. this does not have the requirements so it's not super useful.

I am currently working on comfyui-manager mitigations and it's quite the slog. IMO, I can simply maintain the package fork that is installable, if that is more helpful to you. there are changes to the published repository apparently which are a "full code rewrite," and it's hard for me to keep up with which "rewrites" are relevant or not

I will follow up soon with a better solution. it should just be in-tree, since it is Apache 2.

cjonesuk commented 4 months ago

I saw that there was a full rewrite and it's good if you have another solution anyway. I'm no longer continuing down this path personally. I'll close the ticket. Thanks!

doctorpangloss commented 4 months ago

okay do not hesitate to ticket specific nodes as other users have. since the changes are usually pretty small i think other users have been doing them in an ad-hoc manner, however i would like to improve support for these issues. i am still going to test and merge your changes since they are good. your patience is appreciated.