Skip to content

Conversation

@meiyasan
Copy link

This Pull request:

Hi @hageboeck @vepadulano ; This is a followup on my previous PR #20524 #20539

Changes or fixes:

- Remove unused `fRoots` data member (it was cleared before each root computation and provided no persistent state).
- Return polynomial roots by value instead of via internal storage.
- Mark the affected query/utility methods as `const` to make the class usable in const contexts.
- Apply clang-format to the modified sections.
- Applied @vepadulano suggestions 
- Squashed all commits into one.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Tests:

I ran the ROOT battery of tests with couple of issues.

cmake -DCMAKE_BUILD_TYPE=Debug -Dtesting=ON -Droottest=ON -Dmathmore=ON ../polynomial-patch
make -j8
ctest -j8

Regarding the Polynomial class modifications, my use case output remains unchanged. (I use polynomials to compute digital filtering, biquad, etc..) I also paid attention in particular to those related to polynomials: gtest-hist-hist-testTProfile2Poly, etc..

@meiyasan meiyasan requested a review from lmoneta as a code owner November 27, 2025 01:53
@meiyasan meiyasan force-pushed the polynomial-with-const branch from 5c02ff4 to 162c758 Compare November 27, 2025 11:12
@hageboeck hageboeck self-assigned this Nov 27, 2025
@github-actions
Copy link

github-actions bot commented Nov 27, 2025

Test Results

    22 files      22 suites   4d 2h 10m 31s ⏱️
 3 780 tests  3 362 ✅  0 💤 418 ❌
81 232 runs  80 763 ✅ 50 💤 419 ❌

For more details on these failures, see this check.

Results for commit 81dc8f0.

♻️ This comment has been updated with latest results.

    - Remove unused `fRoots` data member (it was cleared before each root
      computation and provided no persistent state).
    - Return polynomial roots by value instead of via internal storage.
    - Mark the affected query/utility methods as `const` to make the class
      usable in const contexts.
    - Apply clang-format to the modified sections.
@meiyasan meiyasan force-pushed the polynomial-with-const branch from 162c758 to 81dc8f0 Compare November 28, 2025 01:11
Copy link
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants