Proposal: Use Descriptive Variable Names

[wbclark]

Hello,

While looking into some other potential improvements to the project, I noticed a simple opportunity to enhance code readability and maintainability by using more descriptive variable names throughout the project, and figured this would be an easy place to start.

For one example, the variable name cfg is used for configuration in some places. In our context, cfg could sometimes be interpreted as "classifier free guidance" which might cause confusion, so I'd propose config instead.

Another example is that the alias PathT is used in several places but PathType tab-completes just as easily in most editors and is easier to read. :)

If these kinds of minor improvements would be accepted, I'd be happy to create a tracker containing these and other examples and knock them out.

[s1dlx]

hello!

thanks for the PathT PR :)

Regarding cfg I can agree in principle, but that's the name used in hydra docs and I'm used to that convention now. What do you think?

[ghost]

hello!

thanks for the PathT PR :)

Regarding cfg I can agree in principle, but that's the name used in hydra docs and I'm used to that convention now. What do you think?

I looked through the code but couldn't find any use of the webui library except in `install.py'. so, it may be worth considering whether to use webui specifically or as a separate tool.
I believe that if you intend to keep using the name 'sd-webui' for this project, it would be advisable to modify the architecture so that it does not rely on the API and does not use Hydra. Otherwise If you only want to use webui's api, it would be better to develop it as an entirely separate tool.

If you don't make this clear, people will keep asking similar questions and making suggestions.

[s1dlx]

this script works only with webui, through its api. Hence the webui in the name

the GUI is in the making, check out #28

[s1dlx]

Developing a gui is time consuming and it prevents using this script until done. The api is a handy shortcut which allows for testing and merging in a short time.

In my view, the main point of not using the api is to keep the models in memory just after merging. This avoids writing to disk, hence faster optimisation times. This is a lovely goal, but not a high priority one (for me).

Rest assured that there will be a GUI, but with very limited control. That is, hydra is here to stay. The flexibility it allows in configuring the process is difficult to replicate in a GUI. Just think about a GUI with 10 different prompts. It's going to be very difficult to use.

However, if you have a strong reason for using a GUI, please let me know. I'm happy to change my mind

[wbclark]

Regarding cfg I can agree in principle, but that's the name used in hydra docs and I'm used to that convention now. What do you think?

I guess it ultimately depends on your goals for the project. It attempts to build on BWM and address a sore need in the area of quickly finding an optimal mixture of 2 models. If it's intended to grow into something that continues to evolve to meet needs in this area, and is community maintained, then being very deliberate and clear with naming has some small advantages, I think. I suppose it's also a bit friendlier to users who may desire to peek under the hood for learning purposes.

[s1dlx]

I'm not against renaming variables if that helps other understand the code better...

I guess we can collect here the major offenders and do a single renaming PR. For now we have

• self.cfg -> self.config
• PathT -> PathType