ComfyScript icon indicating copy to clipboard operation
ComfyScript copied to clipboard

Enumeration enhancements

Open Praecordi opened this issue 1 year ago • 5 comments

Great work with this project so far. I just got started, and it's working great so far. Can't wait to create some really great stuff with this. I had a quick suggestion. Currently, if a checkpoint, vae, or lora, etc. are located in a file they appear as file_path_{checkpoint/vae/lora/etc}_name. Since these enumerations are created dynamically, would it be possible to represent the file structures as nested enumerations?

Why is this convenient? For example, I, and I'm sure many others organize their checkpoints into folders. Namely, I have a folder for inpainting models, and SDXL and SD1.5 models. I would love to be able to access models with Checkpoints.sd15.dreamshaper8 rather than Checkpoints.sd15_dreamshaper8. The former looks much nicer, and even more importantly, aids in looping, say if I want to loop over all my SD1.5 checkpoints. What do you think?

Praecordi avatar Apr 13 '24 12:04 Praecordi

Yeah, nested enums are in the plan. The reason I didn't make them before is enums cannot be directly nested. For example, the following code doesn't work:

import enum

class Model(enum.Enum):
    ModelA = 'a'
    ModelB = 'b'

    class SDXL(enum.Enum):
        ModelC = 'c'
        ModelD = 'd'

Model.SDXL.ModelC
# AttributeError: 'Model' object has no attribute 'ModelC'

Instead, the child enum must be declared outside of the parent enum, and be accessed through .value:

import enum

class _SDXL(enum.Enum):
   ModelC = 'c'
   ModelD = 'd'

class Model(enum.Enum):
   ModelA = 'a'
   ModelB = 'b'

   SDXL = _SDXL
   
Model.SDXL.value.ModelC
# <_SDXL.ModelC: 'c'>

This is not very ergonomic and I want a better solution. Maybe some third-party libraries like aenum can help.

Chaoses-Ib avatar Apr 13 '24 13:04 Chaoses-Ib

By the way, you can loop over models in a directory without nested enums like this:

for model in Checkpoints:
    if model.startswith('sd15\\'):
        print(model)

# or:
for model in [model for model in Checkpoints if model.startswith('sd15\\')]:
    print(model)

Chaoses-Ib avatar Apr 13 '24 14:04 Chaoses-Ib

Hi, Thanks for the workaround for now. You're right, this is an issue with Python itself. Although is it necessary to use Enums? They're an extension of the base class, so you can probably define a class that acts as an enum. This was suggested in this stackoverflow question. Another suggestion in the same thread was using dataclasses, but I don't see much value for using those in this case.

Praecordi avatar Apr 13 '24 16:04 Praecordi

The main reason Enum is used is to be able to iterate over members, as shown above. Although writing a custom class with __iter__ over __dict__ isn't too complex, using an enum library is still slightly better as it provides more functions.

I've found that Python 3.11 has added a nonmember() decorator that can solve our problem:

import enum

class Model(enum.Enum):
    ModelA = 'a'
    ModelB = 'b'

    @enum.nonmember
    class SDXL(enum.Enum):
        ModelC = 'c'
        ModelD = 'd'

Model.SDXL.ModelC
# <SDXL.ModelC: 'c'>

nonmember is also the default behavior since Python 3.13 (https://github.com/python/cpython/issues/78157). So this is the "standard" solution.

However, requiring 3.11 could be too strict for most users. Using aenum may be the best choice for now.

Chaoses-Ib avatar Apr 13 '24 16:04 Chaoses-Ib

I agree. Switching to 3.11 on my computer would break a lot of existing code for me. And I sorely regret not having some management for my python versions.

I think aenum is the best option as well, if the additional dependency is not a big issue for you.

Praecordi avatar Apr 14 '24 13:04 Praecordi