-
Notifications
You must be signed in to change notification settings - Fork 24
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
Allow square-brackets access to dataset objects #182
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #182 +/- ##
===========================================
+ Coverage 94.26% 94.59% +0.33%
===========================================
Files 35 40 +5
Lines 1325 1758 +433
===========================================
+ Hits 1249 1663 +414
- Misses 76 95 +19 ☔ View full report in Codecov by Sentry. |
Hi Mark, great suggestion - I've implemented something similar in PR #178, but I'm happy for this to be a separate PR as well. |
Hi Mark, Thanks for this addition! To merge this, can you please add an entry to the CHANGELOG.md, as well as update the tests to get the coverage passing. Once that's sorted feel free to request a review from either myself, Nicola, or Martin. It's good to go through this process even on the small PR's. I've opened an issue (#185) to formalise this process for new developers so thanks for prompting that! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the addition! Feel free to merge when you are ready.
Minor fix. PyBOP Dataset class does not currently have a getitem, whereas parts of PyBOP (eg. base_model.py, line 124) attempt to use getitem, which throws an exception.