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

Incompatible with Cython 3 due to division semantics #127

Closed
musicinmybrain opened this issue Jul 21, 2023 · 2 comments · Fixed by #128
Closed

Incompatible with Cython 3 due to division semantics #127

musicinmybrain opened this issue Jul 21, 2023 · 2 comments · Fixed by #128

Comments

@musicinmybrain
Copy link
Contributor

Cython 3.0.0 was released, and indexed_gzip is not quite compatible.

=================================== FAILURES ===================================
________________________________ test_read_all _________________________________
[gw7] linux -- Python 3.12.0 /usr/bin/python3

testfile = '/builddir/build/BUILD/indexed_gzip-1.7.1/ctest_zran_15801045_False.gz'
nelems = 15801045, use_mmap = False

    def test_read_all(testfile, nelems, use_mmap):
        for no_fds in (True, False):
>           ctest_zran.test_read_all(testfile, no_fds, nelems, use_mmap)

../../BUILDROOT/python-indexed_gzip-1.7.1-5.fc39.x86_64/usr/lib64/python3.12/site-packages/indexed_gzip/tests/test_zran.py:86:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   nelemsp = nbytes / 8.
E   TypeError: 'float' object cannot be interpreted as an integer

indexed_gzip/tests/ctest_zran.pyx:799: TypeError
__________________________ test_seek_then_read_block ___________________________
[gw2] linux -- Python 3.12.0 /usr/bin/python3

testfile = '/builddir/build/BUILD/indexed_gzip-1.7.1/ctest_zran_15249998_False.gz'
nelems = 15249998, niters = 1000, seed = 497514082, use_mmap = False

    @pytest.mark.slow_test
    def test_seek_then_read_block(testfile, nelems, niters, seed, use_mmap):
        for no_fds in (True, False):
>           ctest_zran.test_seek_then_read_block(
                testfile, no_fds, nelems, niters, seed, use_mmap
            )

../../BUILDROOT/python-indexed_gzip-1.7.1-5.fc39.x86_64/usr/lib64/python3.12/site-packages/indexed_gzip/tests/test_zran.py:91:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
indexed_gzip/tests/ctest_zran.pyx:817: in indexed_gzip.tests.ctest_zran.test_seek_then_read_block
    with open(testfile, 'rb') as pyfid:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   nelemsp = nbytes / 8.
E   TypeError: 'float' object cannot be interpreted as an integer

indexed_gzip/tests/ctest_zran.pyx:858: TypeError
---------------------------- Captured stdout setup -----------------------------
Seed for random number generator: 497514082
----------------------------- Captured stdout call -----------------------------
0 / 1000: reading 3257996 elements from 9244320 ...

The obvious issue here is that Cython 3 aligns division semantics with Python 3, so x / y always produces a Python float even where x and y are both Python integers. This could be rewritten as x // y to produce an integer.

I could offer a PR for the particular cases showing up in the test failures, but it might be best for someone more familiar with the code to review uses of the / operator throughout the various .pyx files.

@pauldmccarthy
Copy link
Owner

Hi @musicinmybrain, thanks for the report - the only offending division operations are in the unit tests. I'm hoping to have a new release out today, with fixes for this and for #126.

@musicinmybrain
Copy link
Contributor Author

Awesome, thanks!

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.

2 participants