-
Notifications
You must be signed in to change notification settings - Fork 719
[ENH] Add predict
to v2 models
#1984
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1984 +/- ##
=======================================
Coverage ? 86.17%
=======================================
Files ? 162
Lines ? 9600
Branches ? 0
=======================================
Hits ? 8273
Misses ? 1327
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Configs to initialise ``lightning.Trainer``. Defaults to {}. | ||
datamodule_cfg : Union[dict, str, Path], optional | ||
Configs to initialise a ``LightningDataModule``. | ||
- If dict, the keys and values are used as configuration parameters. |
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.
minor formatting issue: please have newlines around bullet point lists
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.
Extremely neat!
I like the design, and I think this is brilliantly done!
Some questions and requests:
- could you write a summary for the PR?
- could you add a vignette somewhere in a notebook or an "examples" section? I think this is crucial to have a full review.
- can you explain how or why the test configs can be replaced by a single test vignette?
- design questions: why are
fit
andpredict
kwargs there and not in__init__
? Have you thought about this?
This PRs adds the
predict
function and related functionality to v2Fixes #1883
PredictCallback
Base_pkg
class andpredict
wrapperpredict
yml
file reading for cfgs (similar topytorch-tabular
)