Skip to content

Conversation

@siliataider
Copy link
Contributor

@siliataider siliataider commented Nov 4, 2025

This Pull request:

Update the histogram tutorials to return void instead of a TCanvas*, which is also the convention used in other ROOT tutorials.

This is required for testing in the Python wheels workflow #19600 since running root tutorial.C in this context returns code 255 , causing pytest to flag the test as failed.

See failure message:

FAILED test/wheels/test_tutorials.py::test_cpp_tutorial[hist004_TH1_labels.C] - subprocess.CalledProcessError: Command '['/opt/hostedtoolcache/Python/3.11.14/x64/bin/root', '-b', '-q', '/opt/hostedtoolcache/Python/3.11.14/x64/lib/python3.11/site-packages/ROOT/tutorials/hist/hist004_TH1_labels.C']' returned non-zero exit status 255.

Checklist:

  • tested changes locally

Copy link
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

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

Tutorials are run as part of the CI, so I would argue the PR should not be marked [skip-ci] and only discover problems after the merge (it happened in the past)

Copy link
Member

@linev linev left a comment

Choose a reason for hiding this comment

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

Also please start CI - some tutorials used for testing, which rely on the fact that non-zero value returned by such tutorial.

@linev
Copy link
Member

linev commented Nov 5, 2025

See this list: https://github.com/root-project/root/blob/master/tutorials/CMakeLists.txt#L547-L562

@siliataider siliataider changed the title [skip-ci][tutorials] Return void instead of TCanvas* in histogram tutorials [tutorials] Return void instead of TCanvas* in histogram tutorials Nov 5, 2025
@siliataider siliataider force-pushed the docs2 branch 2 times, most recently from 81233b5 to 7f1977d Compare November 5, 2025 08:53
@linev linev self-requested a review November 5, 2025 08:53
@linev
Copy link
Member

linev commented Nov 5, 2025

Now looks better - but let see CI first

@github-actions
Copy link

github-actions bot commented Nov 5, 2025

Test Results

    21 files      21 suites   3d 16h 33m 17s ⏱️
 3 707 tests  3 707 ✅ 0 💤 0 ❌
75 983 runs  75 983 ✅ 0 💤 0 ❌

Results for commit c627f1e.

♻️ This comment has been updated with latest results.

@linev linev closed this Nov 5, 2025
@linev linev reopened this Nov 5, 2025
Copy link
Member

@linev linev left a comment

Choose a reason for hiding this comment

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

Now looks ok. Please wait for CI

@siliataider siliataider merged commit d67891a into root-project:master Nov 6, 2025
26 of 27 checks passed
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