Skip to content

feat: add test data from RNTuple Viewer#174

Merged
ariostas merged 3 commits intoscikit-hep:mainfrom
sathabbott:main
Apr 24, 2025
Merged

feat: add test data from RNTuple Viewer#174
ariostas merged 3 commits intoscikit-hep:mainfrom
sathabbott:main

Conversation

@sathabbott
Copy link
Contributor

Adding test data root files from rntviewer.

@eduardo-rodrigues
Copy link
Member

Hello. Let me ping @ariostas as he has been dealing a lot with this package ...

A couple of things to have in mind:

  • By default we want to know what these test files are for, so good to link to the issue that requires a new file for testing, etc. Do not hesitate to add some info more to the header of this PR.
  • We try and follow a lightweight convention to know better what the files are about, see the many examples in the repo already.

@ariostas
Copy link
Member

@sathabbott there's already quite a few RNTuples in here. Do you need anything in specific that is missing, or is it mainly since they are "standard" test files used somewhere else? If it's the latter, maybe it would be good to at least add a rntviewer- prefix to the file name, since RNTuple and multi are not very descriptive.

@codecov
Copy link

codecov bot commented Apr 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.38%. Comparing base (b6f7b74) to head (7fb1ce4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #174   +/-   ##
=======================================
  Coverage   72.38%   72.38%           
=======================================
  Files           3        3           
  Lines         134      134           
=======================================
  Hits           97       97           
  Misses         37       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sathabbott
Copy link
Contributor Author

@eduardo-rodrigues @ariostas
There isn't an issue that I can link to, I made this PR mainly because they are "standard" test files used somewhere else. I can remove the rc2 and rc3 files, and add a rntviewer- prefix.
I will update RNTuple and multi to be more descriptive.

@sathabbott
Copy link
Contributor Author

Please let me know if the test file titles need another update.
Thank you for your time and attention on this!

Copy link
Member

@ariostas ariostas left a comment

Choose a reason for hiding this comment

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

Thank you, @sathabbott! Looks good to me. If @eduardo-rodrigues has no further comments I can merge it and make a release.

@eduardo-rodrigues
Copy link
Member

Hi. Sounds good, thanks to @ariostas's feedback. We're always happy to learn that this test data package is useful outside Scikit-HEP :).

@ariostas ariostas changed the title Adding test data from RNTuple Viewer feat: add test data from RNTuple Viewer Apr 24, 2025
@ariostas ariostas merged commit dc05e44 into scikit-hep:main Apr 24, 2025
7 checks passed
@eduardo-rodrigues
Copy link
Member

eduardo-rodrigues commented Apr 24, 2025

[@ariostas, do not forget to add @sathabbott to the list of contributors, https://github.com/scikit-hep/scikit-hep-testdata?tab=readme-ov-file#contributors. And it seems other people from recent PRs didn't make it to the list either ... and you didn't either, which is odd ;-)!]

@ariostas
Copy link
Member

Oh that's a good point!

@all-contributors please add @sathabbott for data

@allcontributors
Copy link
Contributor

@ariostas

I've put up a pull request to add @sathabbott! 🎉

@sathabbott
Copy link
Contributor Author

Thank you both!

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.

3 participants