Skip to content

Set cython directive binding=True #40341

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

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Jun 30, 2025

Continuation of #26254. This is needed for better integration of Cython functions and thereby unlocks a few further documentation improvements (#27578, #30884, #31309, ...).

Moreover, binding=True is the default in Cython 3.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Jun 30, 2025

Documentation preview for this PR (built with commit a103f87; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

To fix   File "matroid.pyx", line 2698, in init sage.matroids.matroid
NameError: name 'dependent_sets' is not defined
@tobiasdiez
Copy link
Contributor Author

@dcoudert The test for del_vertex that you have added in #39271 (and only that test) fails now with binding=True. Do you have an idea why/how to fix this?

sage -t --warn-long 5.0 --random-seed=162462718596100674006260289682106403625 src/sage/graphs/base/static_sparse_backend.pyx
**********************************************************************
Error: Failed example:: Got: 
Traceback (most recent call last):
  File "/Users/runner/miniconda3/envs/sage-dev/lib/python3.11/site-packages/sage/doctest/forker.py", line 730, in _run
    self.compile_and_execute(example, compiler, test.globs)
  File "/Users/runner/miniconda3/envs/sage-dev/lib/python3.11/site-packages/sage/doctest/forker.py", line 1154, in compile_and_execute
    exec(compiled, globs)
  File "<doctest sage.graphs.base.static_sparse_backend.StaticSparseBackend.del_vertex[3]>", line 1, in <module>
    g.del_vertex('a')
  File "static_sparse_backend.pyx", line 1546, in sage.graphs.base.static_sparse_backend.StaticSparseBackend.del_vertex
TypeError: an integer is required

    g.del_vertex('a')
Expected:
    Traceback (most recent call last):
    ...
    ValueError: graph is immutable; please change a copy instead (use function copy())
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/Users/runner/miniconda3/envs/sage-dev/lib/python3.11/site-packages/sage/doctest/forker.py", line 730, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/runner/miniconda3/envs/sage-dev/lib/python3.11/site-packages/sage/doctest/forker.py", line 1154, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.graphs.base.static_sparse_backend.StaticSparseBackend.del_vertex[3]>", line 1, in <module>
        g.del_vertex('a')
      File "static_sparse_backend.pyx", line 1546, in sage.graphs.base.static_sparse_backend.StaticSparseBackend.del_vertex
    TypeError: an integer is required
**********************************************************************
1 item had failures:
   1 of   9 in sage.graphs.base.static_sparse_backend.StaticSparseBackend.del_vertex
    [198 tests, 1 failure, 0.24s wall]

@dcoudert
Copy link
Contributor

dcoudert commented Jul 1, 2025

Got it, and it's my fault: methods add_vertex and del_vertex are defined twice in StaticSparseBackend.
The solution is to remove the duplicates lines 1512-1546.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants