Skip to content

Conversation

@ramppdev
Copy link
Contributor

There seems to be an issue with the iso-639 package in combination with Poetry (very similar to python-poetry/poetry#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.

@ramppdev ramppdev changed the title Fix iso-639 Poetry Dependeny Issues Fix iso-639 Poetry Dependency Issues Sep 14, 2024
@hagenw
Copy link
Member

hagenw commented Sep 16, 2024

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
Copy link
Member

hagenw commented Sep 16, 2024

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
Copy link
Contributor Author

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
Copy link
Member

hagenw commented Sep 16, 2024

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
Copy link
Contributor Author

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
Copy link
Member

hagenw commented Sep 16, 2024

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
Copy link
Member

hagenw commented Sep 16, 2024

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
Copy link
Member

hagenw commented Sep 16, 2024

I fixed the CI job in #457 and merged it into main, could you rebase your branch.

@ramppdev ramppdev force-pushed the fix/iso-639-dependency branch from 7b802c6 to 1fdcded Compare September 16, 2024 07:51
@ramppdev
Copy link
Contributor Author

Done.

@codecov
Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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 Coverage Δ
audformat/core/utils.py 100.0% <100.0%> (ø)

@hagenw
Copy link
Member

hagenw commented Sep 16, 2024

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.

@hagenw hagenw merged commit cc037b1 into audeering:main Sep 16, 2024
@hagenw hagenw mentioned this pull request Sep 16, 2024
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.

2 participants