feat: ProphetModelDataset #720
Conversation
galenseilis
changed the title
Signed-off-by: galenseilis <galen.seilis@seilis.ca>
|
Hi @galenseilis, thanks for the PR. I think this would be a great candidate to go in as an experimental dataset . This relaxes the requirements for tests. How does that sound? |
Sounds good to me! |
|
Great! @galenseilis are you okay moving it to the experimental directory yourself or do you want me to jump in and make the PR ready for review? |
Hi @merelcht , I would appreciate it if you would jump in to make the PR ready for review. My apologies for any inconvenience. |
|
@galenseilis no worries, happy to help! |
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
|
@galenseilis, I've made all changes required. I've updated the tests and docstrings. The rest are mostly docs build and linting changes. Let me know if all looks good and I'll ask the team to review 🙂 |
There was a problem hiding this comment.
Thank you @galenseilis for the contribution and @merelcht for polishing it!
I left one nit comment but the implementation LGTM.
I approve the PR but as far as I understand it is blocked by #850 now, so we need to resolve it first.
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
There was a problem hiding this comment.
Thanks @galenseilis, I took the liberty to add this to the release notes and fixed bandit related security stuff! Happy to get this in for the next release!
* Added dataset and tests for Facebook's Prophet model * Removed in-path example. * Added prophet as optional dependency in toml * Added prophet as testing dependency * Update docstring to have a doctest example with example data Signed-off-by: galenseilis <galen.seilis@seilis.ca> * Try without assert * Move ProphetModelDataset to experimental datasets Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Move test fixtures + fix imports Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix prophet docstring and tests Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix lint Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix docs Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix docs Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Bandit Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> * Add nosec instead Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> * Add to release notes Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> --------- Signed-off-by: galenseilis <galen.seilis@seilis.ca> Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com> Co-authored-by: Merel Theisen <merel.theisen@quantumblack.com> Co-authored-by: Ankita Katiyar <110245118+ankatiyar@users.noreply.github.com> Co-authored-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> Signed-off-by: Harm Matthias Harms <matthias.harms@quis.de>
* Added dataset and tests for Facebook's Prophet model * Removed in-path example. * Added prophet as optional dependency in toml * Added prophet as testing dependency * Update docstring to have a doctest example with example data Signed-off-by: galenseilis <galen.seilis@seilis.ca> * Try without assert * Move ProphetModelDataset to experimental datasets Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Move test fixtures + fix imports Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix prophet docstring and tests Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix lint Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix docs Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Fix docs Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> * Bandit Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> * Add nosec instead Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> * Add to release notes Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> --------- Signed-off-by: galenseilis <galen.seilis@seilis.ca> Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com> Co-authored-by: Merel Theisen <merel.theisen@quantumblack.com> Co-authored-by: Ankita Katiyar <110245118+ankatiyar@users.noreply.github.com> Co-authored-by: Ankita Katiyar <ankitakatiyar2401@gmail.com> Signed-off-by: Minura Punchihewa <minurapunchihewa17@gmail.com>
Description
This PR was created to add Meta's (formerly Facebook's) Prophet model. It is a probabilistic modelling for time series forecasting.
Development notes
The Prophet module provides functions for serializing and deserializing models to/from JSON. Beyond that the behaviour is intended to be the same as the
JSONDataset. I have subclassed fromJSONDatasetand overrode the_loadand_savemethods to use Prophet's serialization during these methods.I have tried to emulate the tests for
JSONDataset, but with a code duplicate for a trained Prophet model instead of a test file.Checklist
RELEASE.mdfile