Problem in embedding_funcs.py

[cafesao]

Problem ⚠️

I'm running an image from huggingface/transformers-pytorch-gpu and when I download sd_embed and try to add it to the code, I get this error.

Traceback (most recent call last):
   File "app.py", line 10, in <module>
     from sd_embed.embedding_funcs import get_weighted_text_embeddings_sd15
   File "/usr/local/lib/python3.8/dist-packages/sd_embed/embedding_funcs.py", line 1026, in <module>
    pipe: StableCascadePriorPipeline | StableCascadeDecoderPipeline
TypeError: unsupported operand type(s) for |: 'type' and 'type'

Solution 💡

So I just changed | to and.

Test 🧪

Dockerfile

FROM huggingface/transformers-pytorch-gpu:latest

RUN pip install torchao --extra-index-url https://download.pytorch.org/whl/cu121 # full options are cpu/cu118/cu121/cu124
RUN pip install git+https://github.com/cafesao/sd_embed.git@main

WORKDIR /app

CMD ["python3", "app.py"]

Create folder /app and move app.py to folder.

app.py

from sd_embed.embedding_funcs import get_weighted_text_embeddings_sd15

print("Hello Word")

[Teriks]

The problem is that the code uses the Union operator for type hints introduced in python 3.10 https://peps.python.org/pep-0604/ and you are using python 3.8 which does not support it. It could be replaced with typing.Union[StableCascadePriorPipeline, StableCascadeDecoderPipeline] using the typing module and that would probably work

The and operator is not correct for type hints. I am not sure what exactly that does but it evaluates to boolean and for some reason does not raise an error :)

[cafesao]

@Teriks In fact, the image is based on another image that uses Ubuntu 20.04 and which in turn has Python 3.8 pre-installed.

Adding typing.Union[StableCascadePriorPipeline, StableCascadeDecoderPipeline] worked.

I think this is a great addition to the code, as it creates compatibility with previous versions of Python.

If you believe it is not, we can at least add in the README which minimum version of Python is required.

[Teriks]

This fix works for 3.8 compatibility

I think setup.py could have a minimum version constraint just to indicate compatibility, like python_requires='>=3.8'

i.e greater than or equal to version 3.8, so that installing on a version below this presents a useful error.

This mirrors the minimum requirement of the diffusers library which this code is intended to be used with.

It is probably ideal to maintain compatibility with their minimum supported version of python.

3.8 is EOL this October, so it is a reasonable floor.

I do not maintain this repo btw, it is @xhinker, I just saw it was something I contributed causing this :), so just suggestions

[xhinker]

Thank you @Teriks and @cafesao cafesao! I will further look into it later and make changes to fix the problem

[xhinker]

@Teriks are you good with the changes from this PR?

[Teriks]

Seems good!

Other than pipe (StableCascadePriorPipeline and StableCascadeDecoderPipeline) in the function doc string, should probably go back to the | symbol to indicate union.

Otherwise all good

[cafesao]

@Teriks and @xhinker, Perfect, I made the adjustment in the function doc to reflect the same parameters that we passed to the function.

[Teriks]

Good to go @xhinker