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

Manage project with poetry #114

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

piiq
Copy link

@piiq piiq commented Feb 12, 2023

This PR was initially started to resolve #111 by pinning pillow but seeing the work related to #49 done by @christopherwoodall in #108 I add to the discussion by adding dependency and package build management with poetry.

You can install the riffusion package with pip install . and the script interfaces will be riffusion and riffusion-server respectively


Additionally this PR updates the code to support latest version of streamlit

@christopherwoodall
Copy link

christopherwoodall commented Feb 12, 2023

I really like the use of poetry for managing the build/dev environment, and keeping the requirements as a dependency list.

@piiq
Copy link
Author

piiq commented Feb 12, 2023

Great. Thanks 🙏
I can add a github action to publish to pypi next week

@piiq
Copy link
Author

piiq commented Feb 19, 2023

@hmartiro @SethForsgren @christopherwoodall can you have a look at this once you have time?
I've added the action to publish to pypi and it looks like riffusion is still not squated there

@christopherwoodall
Copy link

@piiq if the name is available I would try to grab it as fast as possible. You can always share the repo key with the developers if they decide to publish it.

@hmartiro
Copy link
Member

Aiming to take a look by tomorrow

@hmartiro
Copy link
Member

Hey @piiq , thank you for the work here! I don't have experience with poetry, but I'm concerned that requirements.txt pins all package versions. Most people install with -r requirements.txt and this isn't going to play nice with any other packages in that case is it? Would like help understanding that

requirements.txt Outdated
dora-search==0.1.11 ; python_version >= "3.8" and python_full_version != "3.9.7" and python_version < "3.12"
einops==0.6.0 ; python_version >= "3.8" and python_full_version != "3.9.7" and python_version < "3.12"
entrypoints==0.4 ; python_version >= "3.8" and python_full_version != "3.9.7" and python_version < "3.12"
filelock==3.9.0 ; python_version >= "3.8" and python_full_version != "3.9.7" and python_version < "3.12"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused why these are all locked, when there's already poetry.lock? It seems like this requirements file won't play nice with anyone else

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

poetry.lock is an artifact for poetry that stores info on a fully solved dependency tree. we use the lock file to generate the requirements.txts

@piiq
Copy link
Author

piiq commented Feb 21, 2023

Hey @piiq , thank you for the work here! I don't have experience with poetry, but I'm concerned that requirements.txt pins all package versions. Most people install with -r requirements.txt and this isn't going to play nice with any other packages in that case is it? Would like help understanding that

Warning: wall of text 😂😅

poetry is a tool that can be used to solve the dependency tree and generate the requirements.txt files.

Poetry solves all the deps (including the dependencies of the dependencies of the dependencies) in a way they all play nicely with each other. Poetry uses the top level deps specified in pyproject.toml and generates a poetry.lock artifact with the info on the package version range.
After poetry is done with it's job we can use it and the lock file to produce the requirements.txt files so that the average user doesn't need to know anything about poetry and use the lib the usual way (pip install ...)

So what we're seeing here are auto-generated requirements files that are produced by these 2 commands:

poetry export -f requirements.txt --without-hashes --output requirements.txt
poetry export -f requirements.txt --only dev --without-hashes --output requirements-dev.txt

That's "poetry export dependencies information in a 'requirements.txt format' without hashes into a requirements.txt file"

When you install the dependencies with -r requirements.txt everything should work apart from the fact that you won't have riffusion available for import in your environment namespace and the shortcuts like riffusion-cli and riffusion-server will not work.
To install the package in full you can pip install riffusion and install it from pypi or pip install . from the source code folder.

Now if we consider using riffusion in environments with other dependencies - this would very much depend on the other dependencies. pip itself has some dependency tree resolution mechanism although it's more loose than what we can get with poetry or requirements.txt files so it does the job, but may produce unnecessary and "false positive" warning messages. But it still works.

If you have a specific use case in mind of riffusion not playing nicely with a specific package that's not in the dependency tree - let me know and we'll take a look how we can address that.
If you have any other questions - don't hesitate to ask

@piiq
Copy link
Author

piiq commented Feb 26, 2023

I've tried to fix some linter errors and upgrade the streamlit code to support the new caching methods introduced in 1.18 but the cache gets messed up so it doesn't work nicely with streamlit 1.18+ (and i don't currently have a lot of time to rewire the pages). FYI the problematic point is the deprecated @st.cache decorator of run_img2img
This might require some more rewrite that would definitely not be in scope of making riffusion available through pip.

I have reverted all changes related to the streamlit upgrade and pinned the version to ~1.17.0.
The code from this PR is available for download from pypi with pip install riffusion.

@piiq piiq changed the title Manage project with poetry [WIP] Manage project with poetry Feb 26, 2023
@piiq
Copy link
Author

piiq commented Feb 26, 2023

And mypy gha check is expected to pass now

@piiq piiq changed the title [WIP] Manage project with poetry Manage project with poetry Mar 31, 2023
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.

AttributeError: module 'PIL.Image' has no attribute 'Resampling'
3 participants