-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Issue/pv inputs #225
Issue/pv inputs #225
Conversation
for more information, see https://pre-commit.ci
…cf_datapipes into issue/pv_inputs
for more information, see https://pre-commit.ci
…cf_datapipes into issue/pv_inputs
for more information, see https://pre-commit.ci
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.
Looks really good to me! Thanks for all this, should make the code a lot more readable and easier to understand, as well as better suited for out of UK work.
Super minor comments, and I might also take a stab at the failing spatial slicing. With all the other tests passing, it seems like your changes shouldn't be incorrect, so am curious as to why that might be happening.
Co-authored-by: Jacob Bieker <jacob@bieker.tech>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…cf_datapipes into issue/pv_inputs
for more information, see https://pre-commit.ci
…cf_datapipes into issue/pv_inputs
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## main #225 +/- ##
==========================================
+ Coverage 81.47% 82.45% +0.97%
==========================================
Files 132 125 -7
Lines 5690 5402 -288
==========================================
- Hits 4636 4454 -182
+ Misses 1054 948 -106
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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!
Pull Request
Description
This pull request grew unfortunately organically. But in essence, boils down to
ml_id
which will be consistent across training and database data. I wanted to move aspects like infilling and filtering out of the main loading function to make this more composable#major
Checklist: