Skip to content

[BUG] Raise errors in models.nn if they can't make in-sample and some cases out-sample predictions #813

Merged
merged 6 commits into from
Jul 22, 2022

Conversation

martins0n
Copy link
Contributor

@martins0n martins0n commented Jul 22, 2022

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

Closing issues

closes #787

@github-actions
Copy link

github-actions bot commented Jul 22, 2022

🚀 Deployed on https://deploy-preview-813--etna-docs.netlify.app

@github-actions github-actions bot temporarily deployed to pull request July 22, 2022 12:35 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 22, 2022 12:50 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2022

Codecov Report

Merging #813 (c549ec0) into master (41fcce4) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #813      +/-   ##
==========================================
+ Coverage   84.05%   84.07%   +0.02%     
==========================================
  Files         126      126              
  Lines        7220     7236      +16     
==========================================
+ Hits         6069     6084      +15     
- Misses       1151     1152       +1     
Impacted Files Coverage Δ
etna/models/nn/deepar.py 100.00% <100.00%> (ø)
etna/models/nn/tft.py 100.00% <100.00%> (ø)
etna/core/mixins.py 95.65% <0.00%> (-4.35%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@github-actions github-actions bot temporarily deployed to pull request July 22, 2022 13:18 Inactive
etna/models/nn/tft.py Outdated Show resolved Hide resolved
"You can only forecast from the next point after the last one in the training dataset: "
f"last train timestamp: {self._last_train_timestamp}, first test timestamp is {ts.index[0]}"
)
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this else block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have all possible switches explicitly

Copy link
Contributor

@Mr-Geekman Mr-Geekman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is fine except for < in checking in-sample predictions.

@github-actions github-actions bot temporarily deployed to pull request July 22, 2022 14:58 Inactive
@martins0n martins0n merged commit 66027b9 into master Jul 22, 2022
@Mr-Geekman Mr-Geekman deleted the martins0n/issue787 branch July 22, 2022 16:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Raise errors in models.nn if they can't make in-sample and some cases out-sample predictions
3 participants