Skip to content

[UHI] Exclude TProfile2Poly from the UHI interface#22699

Open
guitargeek wants to merge 1 commit into
root-project:masterfrom
guitargeek:uhi
Open

[UHI] Exclude TProfile2Poly from the UHI interface#22699
guitargeek wants to merge 1 commit into
root-project:masterfrom
guitargeek:uhi

Conversation

@guitargeek

Copy link
Copy Markdown
Contributor

TProfile2Poly derives from TH2Poly, whose bins are polygons addressed by a single global bin number rather than an (i, j) grid. The UHI plotting and indexing pythonizations assume rectangular axes and access bins via the 2-D GetBinContent(i, j) / GetBinContent(*bin) overloads, which are not implemented for TH2Poly and only ever returned a dummy 0.

As a result UHI support for TProfile2Poly was silently meaningless: the indexing and plotting tests compared 0 against 0 and passed vacuously, and a grid-constructed TProfile2Poly has no addressable bins at all (GetNumberOfBins() == 0).

TH2Poly was already deliberately left out of the pythonized class list for this reason; TProfile2Poly had been included by oversight. and should also not be included.

@siliataider siliataider left a comment

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.

Makes sense, thanks!

TProfile2Poly derives from TH2Poly, whose bins are polygons addressed by
a single global bin number rather than an (i, j) grid. The UHI plotting
and indexing pythonizations assume rectangular axes and access bins via
the 2-D GetBinContent(i, j) / GetBinContent(*bin) overloads, which are
not implemented for TH2Poly and only ever returned a dummy 0.

As a result UHI support for TProfile2Poly was silently meaningless: the
indexing and plotting tests compared 0 against 0 and passed vacuously,
and a grid-constructed TProfile2Poly has no addressable bins at all
(`GetNumberOfBins() == 0`).

TH2Poly was already deliberately left out of the pythonized class list
for this reason; TProfile2Poly had been included by oversight. and
should also not be included.
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.

2 participants