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

feat: Add ScoutingNanoAODSchema #1151

Merged
merged 2 commits into from
Aug 14, 2024
Merged

feat: Add ScoutingNanoAODSchema #1151

merged 2 commits into from
Aug 14, 2024

Conversation

jslawless
Copy link
Contributor

This adds a ScoutingNanoAODSchema in src/coffea/nanoevents/schemas/nanoaod.py. Currently this only matches collections in ScoutingNANO to existing mixins in NanoAODSchema. This makes it a bit easier for coffea to work out of the box with scouting data and mc. In the future, the scouting groups could add more scouting specific functionality to this schema.

As a question for the maintainers, should I add a test class along with this new schema in this commit?

@jslawless jslawless marked this pull request as draft August 8, 2024 15:39
@lgray
Copy link
Collaborator

lgray commented Aug 8, 2024

Please add tests!

@jslawless jslawless changed the title Add ScoutingNanoAODSchema feat(wip):Add ScoutingNanoAODSchema Aug 8, 2024
@jslawless jslawless changed the title feat(wip):Add ScoutingNanoAODSchema feat(wip): Add ScoutingNanoAODSchema Aug 8, 2024
@nsmith- nsmith- linked an issue Aug 12, 2024 that may be closed by this pull request
@jslawless
Copy link
Contributor Author

I added a test file "test_nanoevents_scoutingnano.py". I'm not very experienced with pytest so I basically copied what was done in "test_nanoevents_pfnano.py". I had to add a sample file tests/samples/scouting_nano.root. The file I added was ~3.5 MB since I just grabbed the smallest root file I had on hand. This didn't seem to be too far from some of the other root files in that directory but let me know if it needs to be smaller. Is there any guidelines for what should be in sample file or is just a random root file fine?

@jslawless jslawless changed the title feat(wip): Add ScoutingNanoAODSchema feat: Add ScoutingNanoAODSchema Aug 12, 2024
@jslawless jslawless marked this pull request as ready for review August 12, 2024 20:03
@lgray
Copy link
Collaborator

lgray commented Aug 12, 2024

If you can pare down the test file to ~40 events or something like that it'll keep the repository more trim.

@lgray
Copy link
Collaborator

lgray commented Aug 12, 2024

And if you could rebase all your commits into one the larger file won't remain in the repository history.

@jslawless jslawless marked this pull request as draft August 13, 2024 15:36
@jslawless jslawless changed the title feat: Add ScoutingNanoAODSchema feat(wip): Add ScoutingNanoAODSchema Aug 13, 2024
@lgray
Copy link
Collaborator

lgray commented Aug 13, 2024

@jslawless if this is ready to go please mark it as ready for review :-)

…in the ScoutingNANO format.

Add a small test suite for ScoutingNanoAODSchema

Trimmed example root file down to 157K
@jslawless
Copy link
Contributor Author

I changed the tests/sample to only be 157 KB instead of ~3.5 MB. I rebased the commit history so I think the old file is no longer present as well.

@jslawless jslawless marked this pull request as ready for review August 13, 2024 16:46
@jslawless jslawless changed the title feat(wip): Add ScoutingNanoAODSchema feat: Add ScoutingNanoAODSchema Aug 13, 2024
@lgray
Copy link
Collaborator

lgray commented Aug 13, 2024

Looks nice and tidy, thanks!

@lgray
Copy link
Collaborator

lgray commented Aug 14, 2024

@jslawless is there anything else you would like to add to this PR?

@jslawless
Copy link
Contributor Author

This was all I had planned. ScoutingNanoAOD's features aren't frozen yet, so there will be more things to add in the coming months, particularly cross references, but I will add those once the exact format is frozen.

@lgray lgray merged commit 4e8faf5 into CoffeaTeam:master Aug 14, 2024
14 checks passed
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.

Add a Schema for Scouting NanoAOD
2 participants