Skip to content

Try to fix binomial CI failure, take 2#41075

Merged
vbraun merged 2 commits intosagemath:developfrom
user202729:expression-convert-to-bint
Dec 29, 2025
Merged

Try to fix binomial CI failure, take 2#41075
vbraun merged 2 commits intosagemath:developfrom
user202729:expression-convert-to-bint

Conversation

@user202729
Copy link
Copy Markdown
Contributor

@user202729 user202729 commented Oct 19, 2025

try to fix #40978 . I suppose the easiest way to test it is to set it as [CI fix] for a while to see if the issue still happens.

📝 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

@user202729 user202729 force-pushed the expression-convert-to-bint branch 2 times, most recently from 1f33477 to 7f8ace3 Compare October 19, 2025 02:12
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 19, 2025

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

@user202729 user202729 added the p: CI fix merged before running CI tests label Oct 19, 2025
@cxzhong
Copy link
Copy Markdown
Contributor

cxzhong commented Dec 25, 2025

It seems this actually fix the bug. I do not see the error in matrix2 in github ci. Do you consider merge this? Thank you.

@user202729 user202729 marked this pull request as ready for review December 25, 2025 15:05
@user202729
Copy link
Copy Markdown
Contributor Author

user202729 commented Dec 25, 2025

It is likely better for sig_off_then_raise to live in cysignals instead of pynac.pyi, but that can be left for after this function need to be used at another place.

Otherwise should be good.

leading to arbitrary Python code being executed between the ``sig_on()``
and the ``sig_off()``.
"""
sig_off()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems we run twice sig_off()?

finally:
sig_off()
x = g_hold2_wrapper_sig(g_binomial, self._gobj, nexp._gobj, hold)
sig_off()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this line we already have sig_off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentional. Read the comment carefully

@vbraun vbraun merged commit d341a84 into sagemath:develop Dec 29, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: CI fix merged before running CI tests t: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI failure: SystemError: calling remove_from_pari_stack() inside sig_on() in binomial in matrix2.pyx:20953

3 participants