Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug when applying cuts in events_pi.py for certain variable names #699

Closed
kayla-leonard opened this issue May 20, 2022 · 3 comments · Fixed by #792
Closed

Bug when applying cuts in events_pi.py for certain variable names #699

kayla-leonard opened this issue May 20, 2022 · 3 comments · Fixed by #792
Assignees
Labels

Comments

@kayla-leonard
Copy link
Contributor

This chunk of code does not work if one variable_name is a subset of another variable_name:

https://github.com/icecube/pisa/blob/master/pisa/core/events_pi.py#L553-L556

for variable_name in variables:
    crit_str = crit_str.replace(
        variable_name, 'self["%s"]["%s"]' % (key, variable_name)
    )

For example: when trying to load both reco_z and reco_zenith, I end up with expressions like self["nue_cc"]["reco_z"]enith.

@JKrishnamoorthi JKrishnamoorthi self-assigned this Mar 2, 2024
@kayla-leonard
Copy link
Contributor Author

Code to verify behavior:

key = "nue_cc"

crit_str = "(reco_zenith >= 0) & (reco_z >= 500)"
for variable_name in ['reco_z','reco_zenith']:
    crit_str = crit_str.replace(
        variable_name, 'self["%s"]["%s"]' % (key, variable_name)
    )
    
print(crit_str)

currently gives:

(self["nue_cc"]["reco_z"]enith >= 0) & (self["nue_cc"]["reco_z"] >= 500)

but the desired behavior is:

(self["nue_cc"]["reco_zenith"] >= 0) & (self["nue_cc"]["reco_z"] >= 500)

@JKrishnamoorthi
Copy link
Contributor

JKrishnamoorthi commented May 17, 2024

import re

key = "nue_cc"
crit_str = "(reco_zenith >= 0) & (reco_z >= 500)"
for variable_name in ['reco_z','reco_zenith']:
    # Using word boundary \b to replace whole words only
    crit_str = re.sub(r'\b{}\b'.format(variable_name), 'self["%s"]["%s"]' % (key, variable_name), crit_str)

print(crit_str)

A re module with a word boundary expression will solve the issue. If it looks ok, I can make a pull request.

@thehrh
Copy link
Contributor

thehrh commented Jul 31, 2024

Seems like a reasonable low-effort fix to make.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants