clamsproject / clams-python

CLAMS SDK for python
http://sdk.clams.ai/
Apache License 2.0
4 stars 1 forks source link

Creating a MMIF file with clams-source fails #89

Closed marcverhagen closed 1 year ago

marcverhagen commented 2 years ago

When using

$ clams source audio:/audio_in/newshour-99-04-27-short.wav

We get an error because six==0.15.0 is required but six==1.16.0 is installed.

The problem is with what version of six all packages require. Most of them allow any version and some have some minimal requirement that is 1.15.0 or lower. But mmif-python requires 1.15.0. Pip does not seem to realize it can install 1.15.0 and installs 1.16.0 when doing pip install clams-python, it does give you a warning while doing that though.

I am not quite clear on how creating the clams-python pypi package works, but I think that changing the requirements.txt file into the following may help:

mmif-python==0.4.5
Flask==1.1.0
Flask-RESTful==0.3.8
gunicorn==20.1.0
pydantic==1.8.2
jsonschema==3.2.0

This removed the lapps install and moved mmif-python to the front. When I load the above in a clean environment and then pip-install clams-python all is fine.

marcverhagen commented 2 years ago

Another way to deal with this is to have mmif-python require 1.15.0 or higher.

keighrim commented 1 year ago

I have been reading this article: https://bernat.tech/posts/version-numbers/ , and it made a set of valid points not to use upper bounds in the requirements.txt. Here's an excerpt from the article

You’re requesting a version between four and five, while the service library is requesting a version between five and six. Both constraints cannot be satisfied.

If you use a version of pip older than 20.2 ­— the release in which it added a dependency resolver — it will just install a version matching the first constraint it finds and ignore any subsequent constraints. Versions of pip after 20.2 would fail with an error indicating that the constraint cannot be satisfied.

Either way, your application no longer works.

So, as @marcverhagen suggested in the above comment, I think we'd be better off using >= instead of == in the requirements.txt file to address this issue.

marcverhagen commented 1 year ago

Yes, using >= is probably better. It does leave us open to problems where a newer version deprecates a method that we rely on though. I have recently also run into trouble where pip install -r requirements.txt works in Python 3.9 but not 3.10. Maybe it is worth looking at pipenv.

keighrim commented 1 year ago

Regarding the deps-of-deps specifiction (previously mentioned in #100), I think we should leave the resolution up to pip (or other resolver we pick maybe in the future) and only specify explicit (direct) dependencies in our requirements.txt.

marcverhagen commented 1 year ago

We could do both actually. We could specify that you just need to do install pip install clams-python==0.4.1 yet still bundle a requirements file that has version numbers of dependencies.

keighrim commented 1 year ago

I still don't think keeping specific version caps of all deps and deps-of-deps in the requirements.txt file is a good idea. Suppose an app developer uses a library X>=n as well as clams-python==0.4.1 for his/her app, but just by coincidence one of the dependencies of clams-python (0.4.1) has a capped spec on X==m (where m < n) then the app can't be installed in reliable/reasonable way.

marcverhagen commented 1 year ago

Whatever we end up doing or continuing to do with specifying dependencies, the clams source issue that started this conversation is fixed.

keighrim commented 1 year ago

We could do both actually. We could specify that you just need to do install pip install clams-python==0.4.1 yet still bundle a requirements file that has version numbers of dependencies.

We actually can't, because pip install clams-python==0.4.1 will call the bundled setup.py which reads in the bundled req.txt at the install time.


Anyway, #104 and https://github.com/clamsproject/mmif-python/pull/198 both "uncap" the dependency versions, and I believe that should suit better for our current release cycles and CI/build pipelines. We can reconsider capping (or pinning) the entire dependencies (and deps-of-deps) when we are willing to start automated nightly build testing and use more active/aggressive version cycles.

keighrim commented 1 year ago

Closed via https://github.com/clamsproject/clams-python/pull/104 and https://github.com/clamsproject/mmif-python/pull/198 .