Skip to content

Fix tests in preparation for behavior change in modern Cython#10

Merged
rosjat merged 2 commits intopython-scsi:masterfrom
bmeagherix:fix_tests_for_modern_cython
Nov 10, 2025
Merged

Fix tests in preparation for behavior change in modern Cython#10
rosjat merged 2 commits intopython-scsi:masterfrom
bmeagherix:fix_tests_for_modern_cython

Conversation

@bmeagherix
Copy link
Contributor

Modern cython changes how cpdef enums are exposed. Update the tests so that they will work with both old and new cython.

Given

    cpdef enum iscsi_session_type:
        ISCSI_SESSION_DISCOVERY
        ISCSI_SESSION_NORMAL

We used to be able to either use iscsi.ISCSI_SESSION_NORMAL or iscsi.iscsi_session_type.ISCSI_SESSION_NORMAL. With modern cython, only the latter works.

From https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#structs-unions-enums

Up to Cython version 3.0.x, this used to copy all item names into the global module namespace, so that they were available both as attributes of the Python enum type (CheseState above) and as global constants. This was changed in Cython 3.1 to distinguish between anonymous cpdef enums, which only create global Python constants for their items, and named cpdef enums, where the items live only in the namespace of the enum type and do not create global Python constants.

Verified the breakage and fix in a venv on Debian Trixie nightly, and on Bookworm.

The https://pypi.org/project/Cython/ shows

  • 3.1.0 (2025-05-08)

@bmeagherix bmeagherix force-pushed the fix_tests_for_modern_cython branch from 691069c to a7959df Compare May 9, 2025 19:31
@rosjat rosjat requested a review from Flameeyes May 9, 2025 20:04
@rosjat
Copy link
Contributor

rosjat commented May 9, 2025

@Flameeyes can you review and merge on approval please

@rosjat
Copy link
Contributor

rosjat commented Nov 10, 2025

@Flameeyes i will merge this now since it simply bumps the precommit and changes a test case

@rosjat rosjat merged commit 241e987 into python-scsi:master Nov 10, 2025
2 checks passed
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