Repository navigation
Conversation
np.log10(0) produces -inf, and np.floor(-inf) is fine. But on riscv64 the intermediate NaN from the full expression evaluation triggers "RuntimeWarning: invalid value encountered in floor". The existing errstate only suppresses "divide" warnings but not "invalid". This causes TestAsinhLocator tests and test_minorticks_toggle to fail on riscv64.
The default 60-second timeout in subprocess_run_for_testing is too short for riscv64, causing test_sphinxext and test_determinism_check to fail with TimeoutExpired. Increase the default to 300 seconds.
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process. You can also join us on discourse chat for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide. We strive to be a welcoming and open project. Please follow our Code of Conduct. |
subprocess_run_helper already multiplies timeouts by 6 in CI environments, but _test_timeout is used directly by _WaitForStringPopen-based tests (test_sigint, test_other_signal_before_sigint) without going through the helper. Apply the same CI multiplier to _test_timeout for consistency.
|
|
||
|
|
||
| def subprocess_run_for_testing(command, env=None, timeout=60, stdout=None, | ||
| def subprocess_run_for_testing(command, env=None, timeout=300, stdout=None, |
There was a problem hiding this comment.
Can we make this conditional on architecture? The shorter timeout is nicer when working locally so it fails faster if it has failed on arm/x86_64
There was a problem hiding this comment.
There is "if ci environment" thing which bumps timeouts by 6.
This place does not use it so it got patched this way.
May take a look to adapt it next week.
|
Thanks for working on this! |
| # Reasonable safe values for slower CI/Remote and local architectures; timeouts may be | ||
| # automatically increased later by subprocess_run_helper. | ||
| _test_timeout = 20 | ||
| _test_timeout = 120 if is_ci_environment() else 20 |
There was a problem hiding this comment.
As noted in the above comment, this timeout is already increased by subprocess_run_helper.
This branch has not been deployed
3 participants
PR summary
The current hardware for RISC-V architecture is quite slow. Matplotlib tests can be run with "CI=1" which multiplies timeouts by six but there are tests not covered by it. One of changes bumps such place from 60 to 300 seconds.
The other problem is NaN related as RISC-V has other approach to those.
AI Disclosure
LLM was used to analyse build log.
PR quality check