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

Majority of tests fail on Python 3.10 and newer #23

Open
rlaphoenix opened this issue Apr 5, 2023 · 0 comments · May be fixed by #24
Open

Majority of tests fail on Python 3.10 and newer #23

rlaphoenix opened this issue Apr 5, 2023 · 0 comments · May be fixed by #24

Comments

@rlaphoenix
Copy link
Collaborator

rlaphoenix commented Apr 5, 2023

See the following pytest warning and summary information:

tests/test_box.py::BoxTests::test_ftyp_build
  /home/runner/.cache/pypoetry/virtualenvs/pymp4-_EUa4su2-py3.7/lib/python3.7/site-packages/construct/core.py:1000: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3,and in 3.9 it will stop working
    if not isinstance(obj, collections.Sequence):
FAILED tests/test_box.py::BoxTests::test_ftyp_build - AttributeError: module 'collections' has no attribute 'Sequence'
FAILED tests/test_box.py::BoxTests::test_mdhd_build - AttributeError: module 'collections' has no attribute 'Sequence'
FAILED tests/test_box.py::BoxTests::test_mdhd_parse - ValueError: closing stream but 1 unwritten bytes remain, 8 is encoded unit
FAILED tests/test_box.py::BoxTests::test_moov_build - AttributeError: module 'collections' has no attribute 'Sequence'

All of these except one are failing because Python 3.9+ had Sequence move from collections to collections.abc, i.e.:

-from collections import Sequence
+from collections.abc import Sequence

Sequence is never used within pymp4's code, but it is used heavily by construct. This indicates that construct 2.8.8 does not have true support for 3.9+ and upgrading to a newer version of construct is likely required.

It's not immediately clear how we will proceed from here as I'm not sure when construct resolved this internally, and it is unsure if the update that does fix it needs any more fixes.

I would recommend looking into upgrading construct to 2.10.x. Updating to the newest 2.8.x release (2.8.22) doesn't seem to resolve this issue (after doing other changes, see https://github.com/devine-dl/pymp4/tree/construct-2.8.22-patch

A reminder that there is already a pull request by another kind person, #9, which seems to do a lot of the work for upgrading to 2.10.x

EDIT: Confusingly it seems support for Python 3.9 is there, and the warning states it's dropping support in Python 3.10. As of the current master, tests up as far as Python 3.9 do in fact succeed. Not sure where the difference is here. Either way, tests on Python 3.10+ support are definitely broken.

@rlaphoenix rlaphoenix changed the title Majority of tests fail on Python 3.9 and newer Majority of tests fail on Python 3.10 and newer Apr 23, 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 a pull request may close this issue.

1 participant