ndindex icon indicating copy to clipboard operation
ndindex copied to clipboard
verified publisher icon Quansight-Labs

ENH: Free threading support

Open HaoZeke opened this issue 8 months ago • 8 comments

Closes #198. Basically:

  • Uses a critical section for both the global variables in Cython
    • With the slow-and-fast path structuring recommended in the ft-guide
  • Signals to Cython that the code is indeed safe for ft builds
  • Splits the global sections out into their own files since earlier cython versions cannot handle critical_section

Draft until I get the CI right for

  • [ ] testing and
  • [ ] building wheels.

The testing bit is mostly taken from this recent NumPy PR, but since there aren't any tests which use threading (nor AFAIK should there be, the NDIndex objects seem pretty immutable), might not even need the TSAN run.

HaoZeke avatar Apr 24 '25 22:04 HaoZeke

Leaving the thread sanitizer race warnings here for posterity..

Races 
==================
WARNING: ThreadSanitizer: data race (pid=933)
  Read of size 8 at 0x7f0890502298 by main thread:
    #0 PyUnstable_InterpreterFrame_GetLine /tmp/python-build.20250221213836.92/Python-3.13.2/Python/frame.c:152:16 (libpython3.13t.so.1.0+0x3769a8) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #1 PyFrame_GetLineNumber /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/frameobject.c:901:12 (libpython3.13t.so.1.0+0x16368d) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #2 frame_getlineno /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/frameobject.c:907:18 (libpython3.13t.so.1.0+0x16368d)
    #3 getset_get /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/descrobject.c:193:16 (libpython3.13t.so.1.0+0x136dc8) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #4 _PyObject_GenericGetAttrWithDict /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/object.c:1665:19 (libpython3.13t.so.1.0+0x1cd525) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #5 PyObject_GenericGetAttr /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/object.c:1747:12 (libpython3.13t.so.1.0+0x1cd3[72](https://github.com/Quansight-Labs/ndindex/actions/runs/14653425582/job/41124094812?pr=199#step:8:73)) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #6 PyObject_GetAttr /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/object.c:1261:18 (libpython3.13t.so.1.0+0x1cbc97) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #7 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:3766:28 (libpython3.13t.so.1.0+0x3225d3) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #8 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x152aa8) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #9 gen_send_ex2 /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/genobject.c:229:24 (libpython3.13t.so.1.0+0x152aa8)
    #10 gen_iternext /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/genobject.c:589:9 (libpython3.13t.so.1.0+0x150299) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #11 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:2786:24 (libpython3.13t.so.1.0+0x31f0ef) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #12 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x152aa8) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #13 gen_send_ex2 /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/genobject.c:229:24 (libpython3.13t.so.1.0+0x152aa8)
    #14 gen_iternext /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/genobject.c:589:9 (libpython3.13t.so.1.0+0x150299) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #15 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:2786:24 (libpython3.13t.so.1.0+0x31f0ef) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #16 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x314599) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #17 _PyEval_Vector /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval.c:1812:12 (libpython3.13t.so.1.0+0x314599)
    #18 _PyFunction_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c (libpython3.13t.so.1.0+0x122b51) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #19 _PyVectorcall_Call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:2[73](https://github.com/Quansight-Labs/ndindex/actions/runs/14653425582/job/41124094812?pr=199#step:8:74):16 (libpython3.13t.so.1.0+0x1227c3) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #20 _PyObject_Call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:348:16 (libpython3.13t.so.1.0+0x1227c3)
    #21 _PyErr_CheckSignalsTstate /tmp/python-build.20250221213836.92/Python-3.13.2/./Modules/signalmodule.c:1855:22 (libpython3.13t.so.1.0+0x42dbe3) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #22 handle_signals /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval_gil.c:828:9 (libpython3.13t.so.1.0+0x385f87) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #23 _PyEval_MakePendingCalls /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval_gil.c:1045:15 (libpython3.13t.so.1.0+0x385f87)
    #24 Py_MakePendingCalls /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval_gil.c:1073:12 (libpython3.13t.so.1.0+0x385f87)
    #25 ThreadHandle_join /tmp/python-build.20250221213836.92/Python-3.13.2/./Modules/_threadmodule.c:515:17 (libpython3.13t.so.1.0+0x499198) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #26 PyThreadHandleObject_join /tmp/python-build.20250221213836.92/Python-3.13.2/./Modules/_threadmodule.c:643:9 (libpython3.13t.so.1.0+0x4999b3) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #37 _PyObject_VectorcallTstate /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_call.h:166:16 (libpython3.13t.so.1.0+0x12258a) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #38 PyObject_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:327:12 (libpython3.13t.so.1.0+0x12258a)
    #39 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:1502:19 (libpython3.13t.so.1.0+0x31a98b) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #40 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x314599) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #41 _PyEval_Vector /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval.c:1812:12 (libpython3.13t.so.1.0+0x314599)
    #42 _PyFunction_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c (libpython3.13t.so.1.0+0x122b51) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #43 _PyObject_VectorcallDictTstate /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:146:15 (libpython3.13t.so.1.0+0x121680) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #44 _PyObject_Call_Prepend /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:504:24 (libpython3.13t.so.1.0+0x1231a6) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #45 slot_tp_call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/typeobject.c:9539:15 (libpython3.13t.so.1.0+0x22f76a) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #46 _PyObject_MakeTpCall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:242:18 (libpython3.13t.so.1.0+0x12192e) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #47 _PyObject_VectorcallTstate /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_call.h:166:16 (libpython3.13t.so.1.0+0x12258a) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #48 PyObject_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:327:12 (libpython3.13t.so.1.0+0x12258a)
    #49 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:1502:19 (libpython3.13t.so.1.0+0x31a98b) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #50 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x314599) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #51 _PyEval_Vector /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval.c:1812:12 (libpython3.13t.so.1.0+0x314599)
    #52 _PyFunction_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c (libpython3.13t.so.1.0+0x122b51) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #53 _PyObject_VectorcallDictTstate /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:146:15 (libpython3.13t.so.1.0+0x121680) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #54 _PyObject_Call_Prepend /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:504:24 (libpython3.13t.so.1.0+0x1231a6) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #55 slot_tp_call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/typeobject.c:9539:15 (libpython3.13t.so.1.0+0x22f76a) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #56 _PyObject_MakeTpCall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:242:18 (libpython3.13t.so.1.0+0x12192e) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #57 _PyObject_VectorcallTstate /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_call.h:166:16 (libpython3.13t.so.1.0+0x12258a) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #58 PyObject_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:327:12 (libpython3.13t.so.1.0+0x12258a)
    #59 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:1502:19 (libpython3.13t.so.1.0+0x31a98b) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #60 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x314284) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #61 _PyEval_Vector /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval.c:1812:12 (libpython3.13t.so.1.0+0x314284)
    #62 PyEval_EvalCode /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval.c:602:21 (libpython3.13t.so.1.0+0x314284)
    #63 builtin_exec_impl /tmp/python-build.20250221213836.92/Python-3.13.2/Python/bltinmodule.c:1145:17 (libpython3.13t.so.1.0+0x30eb96) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #64 builtin_exec /tmp/python-build.20250221213836.92/Python-3.13.2/Python/clinic/bltinmodule.c.h:556:20 (libpython3.13t.so.1.0+0x30eb96)
    #65 cfunction_vectorcall_FASTCALL_KEYWORDS /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/methodobject.c:441:24 (libpython3.13t.so.1.0+0x1c1853) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #66 _PyObject_VectorcallTstate /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_call.h:168:11 (libpython3.13t.so.1.0+0x1224cc) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #67 PyObject_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:327:12 (libpython3.13t.so.1.0+0x1224cc)
    #68 _PyEval_EvalFrameDefault /tmp/python-build.20250221213836.92/Python-3.13.2/Python/generated_cases.c.h:813:23 (libpython3.13t.so.1.0+0x317d5b) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #69 _PyEval_EvalFrame /tmp/python-build.20250221213836.92/Python-3.13.2/./Include/internal/pycore_ceval.h:119:16 (libpython3.13t.so.1.0+0x314599) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #70 _PyEval_Vector /tmp/python-build.20250221213836.92/Python-3.13.2/Python/ceval.c:1812:12 (libpython3.13t.so.1.0+0x314599)
    #71 _PyFunction_Vectorcall /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c (libpython3.13t.so.1.0+0x122b51) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #72 _PyVectorcall_Call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:273:16 (libpython3.13t.so.1.0+0x1227c3) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #73 _PyObject_Call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:348:16 (libpython3.13t.so.1.0+0x1227c3)
    #[74](https://github.com/Quansight-Labs/ndindex/actions/runs/14653425582/job/41124094812?pr=199#step:8:75) PyObject_Call /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/call.c:373:12 (libpython3.13t.so.1.0+0x122847) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #75 pymain_run_module /tmp/python-build.20250221213836.92/Python-3.13.2/Modules/main.c:349:14 (libpython3.13t.so.1.0+0x40be9a) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #76 pymain_run_python /tmp/python-build.20250221213836.92/Python-3.13.2/Modules/main.c:691:21 (libpython3.13t.so.1.0+0x40b5bb) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #77 Py_RunMain /tmp/python-build.20250221213836.92/Python-3.13.2/Modules/main.c:7[76](https://github.com/Quansight-Labs/ndindex/actions/runs/14653425582/job/41124094812?pr=199#step:8:77):5 (libpython3.13t.so.1.0+0x40b5bb)
    #78 pymain_main /tmp/python-build.20250221213836.92/Python-3.13.2/Modules/main.c:806:12 (libpython3.13t.so.1.0+0x40bc68) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #79 Py_BytesMain /tmp/python-build.20250221213836.92/Python-3.13.2/Modules/main.c:830:12 (libpython3.13t.so.1.0+0x40bcec) (BuildId: 9359595b9a1895ecc20bf063638092944065b5c6)
    #80 main /tmp/python-build.20250221213836.92/Python-3.13.2/./Programs/python.c:15:12 (python3.13+0xe8c9b) (BuildId: 3cc7488ffc292c8dd5400212ea3a107a04537e72)
SUMMARY: ThreadSanitizer: data race /tmp/python-build.20250221213836.92/Python-3.13.2/Objects/object.c:2465:23 in new_reference
==================

HaoZeke avatar Apr 24 '25 23:04 HaoZeke

Rather baffling doc building error, also in CircleCI:

Doc build errors 
/home/runner/work/ndindex/ndindex/docs/indexing-guide/multidimensional-indices/boolean-arrays.md:176: ERROR: Error in "plot" directive:
unknown option: "output-base-name".

.. plot::
   :context: reset
   :include-source: True
   :output-base-name: plot-{counter}
   :alt: A plot of 4*x*np.sin(x) - x**2/4 - 2*x from -10 to 10. The curve crosses the x-axis several times at irregular intervals.
   :caption: Plot of :math:`y = 4x\sin(x) - \frac{x^2}{4} - 2x`

   >>> import matplotlib.pyplot as plt
   >>> x = np.linspace(-10, 10, 10000) # 10000 evenly spaced points between -10 and 10
   >>> y = 4*x*np.sin(x) - x**2/4 - 2*x # our function
   >>> plt.scatter(x, y, marker=',', s=1)
   <matplotlib.collections.PathCollection object at ...> [docutils]
/home/runner/work/ndindex/ndindex/docs/indexing-guide/multidimensional-indices/boolean-arrays.md:194: ERROR: Error in "plot" directive:
unknown option: "output-base-name".

.. plot::
   :context: close-figs
   :include-source: True
   :output-base-name: plot-{counter}
   :alt: A plot of only the parts of 4*x*np.sin(x) - x**2/4 - 2*x that are above the x-axis.
   :caption: Plot of :math:`y = 4x\sin(x) - \frac{x^2}{4} - 2x` where :math:`y > 0`

   >>> plt.scatter(x[y > 0], y[y > 0], marker=',', s=1)
   <matplotlib.collections.PathCollection object at ...> [docutils]
/home/runner/work/ndindex/ndindex/docs/indexing-guide/multidimensional-indices/boolean-arrays.md:387: ERROR: Error in "plot" directive:
unknown option: "output-base-name".

.. plot::
   :context: reset
   :include-source: True
   :output-base-name: astronaut-{counter}
   :alt: An image of an astronaut, which is represented as a shape (512, 512, 3) array.

   >>> def imshow(image, title):
   ...     import matplotlib.pyplot as plt
   ...     plt.axis('off')
   ...     plt.title(title)
   ...     plt.imshow(image)
   >>> from skimage.data import astronaut
   >>> image = astronaut()
   >>> image.shape
   (512, 512, 3)
   >>> imshow(image, "Original Image") [docutils]
/home/runner/work/ndindex/ndindex/docs/indexing-guide/multidimensional-indices/boolean-arrays.md:[41](https://github.com/Quansight-Labs/ndindex/actions/runs/14654158449/job/41126132114?pr=199#step:6:42)0: ERROR: Error in "plot" directive:
unknown option: "output-base-name".

HaoZeke avatar Apr 25 '25 00:04 HaoZeke

Test failures are actually coverage issues so far (TSAN aside).

HaoZeke avatar Apr 25 '25 00:04 HaoZeke

The docs build error is because the docs require https://github.com/matplotlib/matplotlib/pull/28187. I thought I had the CI set up to use that PR when building.

asmeurer avatar Apr 25 '25 03:04 asmeurer

the NDIndex objects seem pretty immutable

NDIndex objects are all designed to be 100% immutable. Even the objects that wrap arrays (which aren't yet Cythonized) copy them and set them to be read-only so that they can be immutable and hashable.

asmeurer avatar Apr 25 '25 03:04 asmeurer

Oh, regarding the docs thing, I guess I changed the option name in the matplotlib PR and never updated it here.

asmeurer avatar Apr 25 '25 03:04 asmeurer

I'm not sure whether the TSAN tests are hanging or if they're just really slow. I would avoid TSAN if it adds a ton of overhead to the tests.

ASAN will likely be a lot faster, although it won't catch as many things. It will catch serious issues, though.

ngoldbaum avatar Apr 25 '25 14:04 ngoldbaum

The docs build should be fixed on main.

asmeurer avatar Apr 25 '25 17:04 asmeurer

A sanity check for the CI approach here: I did not expect a TSan job to be added, or ASan. We haven't done that in any project except for NumPy (and maybe PyO3?). It's probably only going to catch things in other projects (CPython, NumPy, Cython) and be worth way more trouble than it's worth - all I'd expect is future maintenance headaches. The way I'd deal with this is to run it once in a Docker container, which is quick and easy with https://github.com/nascheme/cpython_sanity, and if it turns up nothing then check the box that it was clean in a comment or the PR description here.

The standard approach to testing is to add a single cp313t job with pytest-run-parallel, which is a lot easier to do and maintain.

rgommers avatar May 02 '25 19:05 rgommers

This is starting to look close. call_once is new in Cython 3.1 so needs protecting based on version number.

I'd also suggest putting the unrelated minor changes like removing the classifier from setup.py and the .gitignore/.coveragerc tweaks in a separate PR that can be merged straight away.

rgommers avatar May 05 '25 06:05 rgommers

@HaoZeke I thought the Cython code was already fixed, with a nice and small diff. Why is it now completely duplicated again? I think all you needed was:

if Version(cython_version) >= Version("3.1.0a0"):
    compiler_directives["freethreading_compatible"] = True

The comment-directives at the top of the .pyx files are also unnecessary.

rgommers avatar May 06 '25 07:05 rgommers

@HaoZeke I thought the Cython code was already fixed, with a nice and small diff. Why is it now completely duplicated again? I think all you needed was:

if Version(cython_version) >= Version("3.1.0a0"):
    compiler_directives["freethreading_compatible"] = True

The comment-directives at the top of the .pyx files are also unnecessary.

The diff for working with free threading is nice and small. The duplication is for BC because the resulting file cannot be compiled with older Cython versions.

i.e.:

Successfully installed cython-3.0.12 setuptools-80.3.1
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Running command Getting requirements to build wheel

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
  # distutils: language = c++

  import sys
  from _import_cache import _NUMPY_IMPORTED, _NUMPY_BOOL, _NUMPY_NDARRAY

  from libcpp.mutex cimport once_flag, call_once
  ^
  ------------------------------------------------------------

  ndindex/_tuple.pyx:7:0: 'libcpp/mutex.pxd' not found

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
  # distutils: language = c++

  import sys
  from _import_cache import _NUMPY_IMPORTED, _NUMPY_BOOL, _NUMPY_NDARRAY

  from libcpp.mutex cimport once_flag, call_once
  ^
  ------------------------------------------------------------

  ndindex/_tuple.pyx:7:0: 'libcpp/mutex/once_flag.pxd' not found

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
  # distutils: language = c++

  import sys
  from _import_cache import _NUMPY_IMPORTED, _NUMPY_BOOL, _NUMPY_NDARRAY

  from libcpp.mutex cimport once_flag, call_once
  ^
  ------------------------------------------------------------

  ndindex/_tuple.pyx:7:0: 'libcpp/mutex/call_once.pxd' not found

  Error compiling Cython file:
  ------------------------------------------------------------
  ...

  # We cannot just add these imports to the top because of circular import
  # issues. We can put them inside the constructor, but then they create a big
  # performance bottleneck.
  # Uses C++ primitives for preventing races
  cdef once_flag _lazy_import_lock
       ^
  ------------------------------------------------------------

  ndindex/_tuple.pyx:[61](https://github.com/Quansight-Labs/ndindex/actions/runs/14826373995/job/41619982673#step:5:62):5: 'once_flag' is not a type identifier

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
          _broadcast_shapes = broadcast_shapes
          _BroadcastError = BroadcastError

  cdef int _is_boolean_scalar(object idx):
      cdef object BooleanArray
      call_once(_lazy_import_lock, _lazy_import)
      ^
  ------------------------------------------------------------

  ndindex/_tuple.pyx:83:4: 'call_once' is not a constant, variable or function identifier

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
              int array_block_stop = 0
              int has_array = 0
              int has_boolean_scalar = 0
              object arg, newarg

          call_once(_lazy_import_lock, _lazy_import)
          ^
  ------------------------------------------------------------

  ndindex/_tuple.pyx:103:8: 'call_once' is not a constant, variable or function identifier

and similar from the CI.

The fix (which unfortunately duplicates the code) is to have conditional compilation based on the Cython version, which is done in this commit with:

if Version(cython_version) >= Version("3.1.0a0"):
    compiler_directives["freethreading_compatible"] = True
    _tuple = Extension(
        "ndindex._tuple", ["ndindex/_tuple_ft.pyx"],
        define_macros=define_macros,
    )
else:
    _tuple = Extension(
        "ndindex._tuple", ["ndindex/_tuple_gil.pyx"],
        define_macros=define_macros,
    )

HaoZeke avatar May 06 '25 07:05 HaoZeke

The fix (which unfortunately duplicates the code) is to have conditional compilation based on the Cython version, which is done in this commit with:

The difference between the files is:

1c1,3
< # cython: freethreading_compatible = False
---
> # cython: freethreading_compatible = True
> # distutils: language = c++
> 
3a6
> from libcpp.mutex cimport once_flag, call_once
13c16,18
< cdef void _lazy_import():
---
> # Uses C++ primitives for preventing races
> cdef once_flag _lazy_import_lock
> cdef void _lazy_import() noexcept with gil:
34c39,40
<     _lazy_import()
---
>     with nogil:
>         call_once(_lazy_import_lock, _lazy_import)
54c60,61
<         _lazy_import()
---
>         with nogil:
>             call_once(_lazy_import_lock, _lazy_import)

The # cython don't need to be different (use c++ mode always, and the free-threading directives are unnecessary because it's handled in setup.py already). So you're left with only a couple of lines. Duplicating everything for that feels wrong - a couple of ideas:

  • Put the common code in a _tuple.pxi and include that (see https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#the-include-statement-and-include-files)
  • Use IF (see https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#conditional-statements). Yes it's deprecated, and no that isn't important - it can be removed again in a few months when requiring Cython>=3.1.0, and it won't go away any time soon. Example from pandas: https://github.com/pandas-dev/pandas/pull/60540).

rgommers avatar May 06 '25 08:05 rgommers

  • Put the common code in a _tuple.pxi and include that (see https://cython.readthedocs.io/en/latest/src/userguide/language_basics.html#the-include-statement-and-include-files)
  • Use IF (see

Good point, I managed to clean it up to minimize differences to main. AFAIK .pxi isn't good here since the code to be swapped out is part of a member function. The IF conditional compilation directive seems to be the only way through here since normal try-catch won't take cdef, might be something to point out to https://github.com/cython/cython/issues/4310.

HaoZeke avatar May 06 '25 20:05 HaoZeke

Cython 3.1 came out today so all the special handling for that can be deleted.

ngoldbaum avatar May 09 '25 17:05 ngoldbaum

I'm a bit flummoxed as to why the changes since https://github.com/Quansight-Labs/ndindex/pull/199/commits/24ae26c980196cf5713982562b509598ebcf7534 are causing so many coverage failures.

HaoZeke avatar May 09 '25 20:05 HaoZeke

Half of this PR is now # pragma: no cover but other than that I guess its ready to go.

For reference, before this set of skips : https://github.com/Quansight-Labs/ndindex/pull/199/commits/3967b5d26da0db49b32ce6a77769d1dcef96b8c3

The only skips needed for https://github.com/Quansight-Labs/ndindex/pull/199/commits/24ae26c980196cf5713982562b509598ebcf7534 were https://github.com/Quansight-Labs/ndindex/pull/199/commits/f91f81e9d6fda37218084db227b6432f610566b1 and https://github.com/Quansight-Labs/ndindex/pull/199/commits/e8c9574226942c7149c7dc36007ba66f260fce59

HaoZeke avatar May 09 '25 21:05 HaoZeke

Half of this PR is now # pragma: no cover

It's happening on main as well (see gh-202), so it's unrelated to your changes it looks like. Adding a ton of pragma's can't be the right solution, it has to be some dependency that changed. Both hypothesis and Cython did releases on May 8th, so right before things started failing.

rgommers avatar May 12 '25 09:05 rgommers

We should replace 3.13-dev with just 3.13 in the CI (we could also add 3.14-dev, but that can be done in a separate PR).

asmeurer avatar May 12 '25 16:05 asmeurer

We should replace 3.13-dev with just 3.13 in the CI (we could also add 3.14-dev, but that can be done in a separate PR).

I'll take this along too.

rgommers avatar May 13 '25 05:05 rgommers

It's too early for 3.14-dev by the way: that attempts to build numpy, scikit-image and maybe more things from source.

rgommers avatar May 13 '25 05:05 rgommers

Oh actually, it needs some history rewriting since there's no squash-merge enabled. Let me just do that right now and get this over the finish line.

rgommers avatar May 14 '25 10:05 rgommers