audeering / audformat

Format to store media files and annotations
https://audeering.github.io/audformat/
Other
11 stars 1 forks source link

Fix iso-639 Poetry Dependency Issues #456

Closed ramppdev closed 2 months ago

ramppdev commented 2 months ago

There seems to be an issue with the iso-639 package in combination with Poetry (very similar to https://github.com/python-poetry/poetry/issues/6996). Poetry is unable to determine the version of the package (see below), therefore audformat as well as any packages that depend on it (like opensmile) cannot be used in poetry projects.

Because test-audformat depends on iso-639 (*) which doesn't match any versions, version solving failed.

To reproduce, run poetry lock for the following pyproject.toml:

[tool.poetry]
name = "test-audformat"
version = "0.1.0"
description = ""
authors = []

[tool.poetry.dependencies]
python = "^3.9"
iso-639 = "^0.4.5"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

The workaround for local projects is to directly fetch the package from git:

iso-639 = { git = "https://github.com/noumar/iso639.git", tag = "0.4.5", optional = true }

However, this is not a viable solution for publishing on PyPI, i.e. no package can be published using poetry that depends on audformat.


This PR replaces iso-639 with iso639-lang under the hood to solve the dependency resolution. All tests are passing and running poetry lock with the updated dependency works like a charm.

hagenw commented 2 months ago

Great, seems anyway a good idea to find a replacement for iso-639 as it latest release is from 2016.

Did you do any comparison between https://github.com/LBeaudoux/iso639 and https://github.com/jacksonllee/iso639, or is it just random that you picked iso639-lang?

hagenw commented 2 months ago

Currently, the tests fail with:

image

see https://github.com/audeering/audformat/actions/runs/10865503635/job/30182697617?pr=456.

Locally, I can reproduce the error.

ramppdev commented 2 months ago

Great, seems anyway a good idea to find a replacement for iso-639 as it latest release is from 2016.

Did you do any comparison between https://github.com/LBeaudoux/iso639 and https://github.com/jacksonllee/iso639, or is it just random that you picked iso639-lang?

I have only looked at both of their READMEs and decided to go with https://github.com/LBeaudoux/iso639 as it was updated more recently.

Currently, the tests fail with:

image

see https://github.com/audeering/audformat/actions/runs/10865503635/job/30182697617?pr=456.

Locally, I can reproduce the error.

Interesting, i am unable to reproduce this on Windows. Pretty sure this means that the wrong library is used.

hagenw commented 2 months ago

I tested locally under Linux.

Pretty sure this means that the wrong library is used.

When I start from an empty virtual environment, I don't get the error.

Locally, I could reproduce the error as I first installed iso639-lang, and then removed iso-639, so it seems not a good idea if packages use the exact same name for the module. I guess the problem in the Ci jobs might be that the virtual environment is cached.

ramppdev commented 2 months ago

I tested locally under Linux.

Pretty sure this means that the wrong library is used.

When I start from an empty virtual environment, I don't get the error.

Locally, I could reproduce the error as I first installed iso639-lang, and then removed iso-639, so it seems not a good idea if packages use the exact same name for the module. I guess the problem in the Ci jobs might be that the virtual environment is cached.

Exactly, also just verified that this is what happens. However the issue is that all packages use iso639.

hagenw commented 2 months ago

When running pip install -r requirements.txt in the CI pipeline it installs:

image

The problem is that then it runs pip install -r tests/requirements.txt which installs

image

For whatever reason it seems not satisfied with the dev version of audformat when installing tests/requirements.txt, but installs audformat 1.3.0, which has iso-639 as a dependency.

hagenw commented 2 months ago

It seems that when installing the dev version of audformat the version is wrong:

image

It should instead be something like 1.3.1.dev1+g7b802c6. I guess the version of the cloned repository is to shallow and does not contain enough information to define the correct version. I will try to fix this in another pull request.

hagenw commented 2 months ago

I fixed the CI job in https://github.com/audeering/audformat/pull/457 and merged it into main, could you rebase your branch.

ramppdev commented 2 months ago

Done.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.0%. Comparing base (20ecec3) to head (1fdcded). Report is 1 commits behind head on main.

Additional details and impacted files | [Files with missing lines](https://app.codecov.io/gh/audeering/audformat/pull/456?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering) | Coverage Δ | | |---|---|---| | [audformat/core/utils.py](https://app.codecov.io/gh/audeering/audformat/pull/456?src=pr&el=tree&filepath=audformat%2Fcore%2Futils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkZm9ybWF0L2NvcmUvdXRpbHMucHk=) | `100.0% <100.0%> (ø)` | |
hagenw commented 2 months ago

All looks fine now, thanks again for your contribution. I will merge now and later make a new audformat release, so you can benefit from the change.