-
Notifications
You must be signed in to change notification settings - Fork 268
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
Write out cross-validation results immediately #2483
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2483 +/- ##
==========================================
+ Coverage 92.44% 92.47% +0.02%
==========================================
Files 234 234
Lines 19917 19965 +48
==========================================
+ Hits 18413 18462 +49
+ Misses 1504 1503 -1 ☔ View full report in Codecov by Sentry. |
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.
Good start, but you should
a) Check overwrite conditions in __init__
: if output file exists and not overwrite, raise an error, if output file exists and overwrite=true truncate it (tables.open_file(path, mode='w').
b) Move the writing into the cv loop using write_table(..., append=True)
to further reduce memory usage
c) Add writing of metadata to the output file
Fix tools after removing unused argument
260c85f
to
ceef17a
Compare
a) The error if file exists and not overwrite is already raised by the train tools via a)/b) Using c) Is there any other metadata requiered besides some column descriptions? |
Also for the cv output file and not just the model? |
You need to truncate the file once in |
Yes, both output paths are passed and |
Fixes #2480