-
Notifications
You must be signed in to change notification settings - Fork 80
Conversation
🚀 Deployed on https://deploy-preview-753--etna-docs.netlify.app |
Codecov Report
@@ Coverage Diff @@
## master #753 +/- ##
==========================================
- Coverage 83.66% 83.47% -0.20%
==========================================
Files 120 122 +2
Lines 6631 6736 +105
==========================================
+ Hits 5548 5623 +75
- Misses 1083 1113 +30
📣 Codecov can now indicate which changes are the most critical in Pull Requests. 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.
Well done, but need some fixes👍
- Update the changelog
- Link the issue to the PR in "Closing issues" section like this "Closes #."
- Assign yourself to the issue and to the PR
- Fix all the comments I left
Also, it is a good practice to set more informative title to the PR to make it easier to guess what was this PR about
CHANGELOG.md
Outdated
@@ -449,4 +449,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
- Distribution plot | |||
- Anomalies (Outliers) plot | |||
- Backtest (CrossValidation) plot | |||
- Forecast plot | |||
- Forecast plot |
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.
I guess when you resolve conflicts this will disappear, but it is strange
etna/analysis/plotters.py
Outdated
* spearman: Spearman rank correlation | ||
|
||
mode: 'macro' or 'per-segment' | ||
aggregation mode |
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.
Start the description with the capital letter
We actually don't have a convention about it, however all of them should start with the same type of the letter
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 good👍
- Fix the docstring in one more place
- Also you need to resolve conflicts in Changelog, you can do it with the GitHub UI
IMPORTANT: Please do not create a Pull Request without creating an issue first.
Before submitting (must do checklist)
Type of Change
Proposed Changes
Related Issue
Closing issues
closes #726