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

fix: added hasattr check to NanoAODEvents __repr__ #909

Merged
merged 20 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/coffea/nanoevents/methods/nanoaod.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@

class _NanoAODEvents(behavior["NanoEvents"]):
def __repr__(self):
return f"<event {self.run}:{self.luminosityBlock}:{self.event}>"
if (
hasattr(self, "run")
and hasattr(self, "luminosityBlock")
and hasattr(self, "event")
):
Copy link
Collaborator

@NJManganelli NJManganelli Oct 16, 2023

Choose a reason for hiding this comment

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

Another option may be to use getattr(self, field, "??") such that if 1 or 2 of the fields are there, they'll still print. (Or something similar)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the attributes are missing I don't know if it is better just to print an empty string or again to warn the user that the attributes are missing with something like

"<event (missing run):(missing luminosityBlock):(missing Event)>"

Copy link
Collaborator

Choose a reason for hiding this comment

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

repeated warnings tend to be annoying to users, for what it's worth.

return f"<event {self.run}:{self.luminosityBlock}:{self.event}>"
else:
return "<event>"


behavior["NanoEvents"] = _NanoAODEvents
Expand Down
24 changes: 24 additions & 0 deletions src/coffea/nanoevents/schemas/nanoaod.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,16 @@ class NanoAODSchema(BaseSchema):
There is a class-level variable ``warn_missing_crossrefs`` which will alter the behavior of
NanoAODSchema. If warn_missing_crossrefs is true then when a missing global index cross-ref
target is encountered a warning will be issued. Regardless, the cross-reference is dropped.

The same holds for ``error_missing_events_id``. If error_missing_events_id is true, then when the 'run', 'event',
or 'luminosityBlock' fields are missing, an exception will be thrown; if it is false, just a warning will be issued.
"""

__dask_capable__ = True
warn_missing_crossrefs = True
error_missing_event_ids = True

event_ids = ["run", "luminosityBlock", "event"]
pviscone marked this conversation as resolved.
Show resolved Hide resolved

mixins = {
"CaloMET": "MissingET",
Expand Down Expand Up @@ -206,6 +212,24 @@ def _build_collections(self, field_names, input_contents):
branch_forms["n" + name]
)

# Check the presence of the event_ids
missing_event_ids = [
event_id for event_id in self.event_ids if event_id not in branch_forms
]
if missing_event_ids:
pviscone marked this conversation as resolved.
Show resolved Hide resolved
if self.error_missing_event_ids:
raise Exception(
f"event_ids {missing_event_ids} is/are missing. "
pviscone marked this conversation as resolved.
Show resolved Hide resolved
"These fields can be needed to distinguish problematic "
"data collection sub-eras or to do an analysis cross-check.\n"
"To prevent this error turn error_missing_event_ids to False"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these 3 lines can read as follows (@andrzejnovak @lgray @nsmith- crosscheck me on verbosity):
"The event ID fields {self.event_ids} are necessary to perform sub-run identification (e.g. for corrections and sub-dividing data during different detector conditions), to cross-validate MC and Data (i.e. matching events for comparison), and to generate event displays. It's advised to never drop these branches from the dataformat.

This error can be demoted to a warning by setting the class level variable error_missing_event_ids to False."

)
else:
warnings.warn(
f"Missing event_ids : {missing_event_ids}",
RuntimeWarning,
)

# Create global index virtual arrays for indirection
for indexer, target in self.cross_references.items():
if target.startswith("Gen") and isData:
Expand Down