[treeplayer] Don't mistake formulas for script files in TTree::Draw#22774
Open
guitargeek wants to merge 1 commit into
Open
[treeplayer] Don't mistake formulas for script files in TTree::Draw#22774guitargeek wants to merge 1 commit into
guitargeek wants to merge 1 commit into
Conversation
`TTree::Draw` can take the name of a C++ script file instead of a
TTreeFormula expression for both the variable expression and the
selection. To decide between the two, `TTreePlayer::DrawSelect` checked
whether a matching file exists via `TSystem::IsFileInIncludePath()`,
which strips a trailing parenthesized part as ACLiC-style macro
arguments. A formula like `"abs(Bs_TRUEID)"` was thus reduced to
`"abs"`, and if an unrelated file with that name happened to exist in
the current directory, the Draw call failed:
```
root [0] tree->Draw("Bs_MCORR", "abs(Bs_TRUEID)")
Error in <TTreePlayer::DrawSelect>: Drawing using a C++ file
currently requires that both the expression and the selection are
files: "Bs_MCORR" is not a file
```
Two inconsistencies made the trap wider: the selection branch did not
require a dot in the candidate string at all, and the varexp branch
checked for a dot before stripping the arguments, so a decimal
constant inside the parentheses (e.g. `"abs(x+0.5)"`) satisfied it.
Factor the decision into a single helper used for both arguments, which
requires the candidate - after stripping the ACLiC mode and arguments -
to have an extension. This is not a new restriction:
`TTreeProxyGenerator::WriteProxy()` already refuses a script without an
extension (the function name is derived from the basename minus the
extension, for the cut script too), so such a file could never be
processed anyway. It now evaluates as a formula instead of failing with
a misleading error.
Add a regression test that exercises the previously failing Draw
calls in the presence of a shadowing file named "abs".
Fixes https://its.cern.ch/jira/browse/ROOT-8000
🤖 Done with the help of AI
Test Results 23 files 23 suites 3d 17h 4m 59s ⏱️ For more details on these failures, see this check. Results for commit 5fc9c32. ♻️ This comment has been updated with latest results. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TTree::Drawcan take the name of a C++ script file instead of a TTreeFormula expression for both the variable expression and the selection. To decide between the two,TTreePlayer::DrawSelectchecked whether a matching file exists viaTSystem::IsFileInIncludePath(), which strips a trailing parenthesized part as ACLiC-style macro arguments. A formula like"abs(Bs_TRUEID)"was thus reduced to"abs", and if an unrelated file with that name happened to exist in the current directory, the Draw call failed:Two inconsistencies made the trap wider: the selection branch did not require a dot in the candidate string at all, and the varexp branch checked for a dot before stripping the arguments, so a decimal constant inside the parentheses (e.g.
"abs(x+0.5)") satisfied it.Factor the decision into a single helper used for both arguments, which requires the candidate - after stripping the ACLiC mode and arguments - to have an extension. This is not a new restriction:
TTreeProxyGenerator::WriteProxy()already refuses a script without an extension (the function name is derived from the basename minus the extension, for the cut script too), so such a file could never be processed anyway. It now evaluates as a formula instead of failing with a misleading error.Add a regression test that exercises the previously failing Draw calls in the presence of a shadowing file named "abs".
Fixes https://its.cern.ch/jira/browse/ROOT-8000
🤖 Done with the help of AI