-
Notifications
You must be signed in to change notification settings - Fork 2
feat(data structures, streams) moving average #131
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
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new moving-average family (MovingAverage, MovingAverageWithBuffer, ExponentialMovingAverage, WeightedMovingAverage) with docs and tests, and introduces a CircularBuffer implementation with explicit exceptions and package-level re-exports; updates package init exports and removes a duplicate Test Circular Buffer entry from DIRECTORY.md. Changes
Sequence Diagram(s)(Skipped — changes are self-contained library/data-structure implementations without multi-component runtime interactions.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
datastructures/streams/moving_average/test_moving_average.py (1)
22-22: Remove redundant statement.Line 22 calls
round(actual, 5)but doesn't use the result. The rounding is already performed in the assertion on line 23, making this line unnecessary.🔎 Proposed fix
for data, expected in data_to_expected: actual = moving_average.next(data) - round(actual, 5) self.assertEqual(expected, round(actual, 5))DIRECTORY.md (1)
328-329: Consider adjusting indentation for markdown linting consistency.The markdown linter expects 2 spaces for the first-level list item and 4 spaces for the nested item, but the current indentation uses 4 and 6 spaces respectively. While this doesn't affect functionality, adjusting it would align with the linting rules and maintain consistency with the rest of the file.
🔎 Proposed fix
- * Moving Average - * [Test Moving Average](https://github.com/BrianLusina/PythonSnips/blob/master/datastructures/streams/moving_average/test_moving_average.py) + * Moving Average + * [Test Moving Average](https://github.com/BrianLusina/PythonSnips/blob/master/datastructures/streams/moving_average/test_moving_average.py)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
datastructures/streams/moving_average/images/examples/moving_average_example_1.pngis excluded by!**/*.png
📒 Files selected for processing (4)
DIRECTORY.mddatastructures/streams/moving_average/README.mddatastructures/streams/moving_average/__init__.pydatastructures/streams/moving_average/test_moving_average.py
🧰 Additional context used
🧬 Code graph analysis (1)
datastructures/streams/moving_average/test_moving_average.py (1)
datastructures/streams/moving_average/__init__.py (2)
MovingAverage(5-34)next(16-34)
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
328-328: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
329-329: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🔇 Additional comments (4)
datastructures/streams/moving_average/README.md (1)
1-62: LGTM! Well-structured documentation.The documentation is comprehensive, clearly explaining the problem, solution approach, and complexity analysis. The step-by-step algorithm explanation aligns well with the implementation.
datastructures/streams/moving_average/__init__.py (2)
1-2: LGTM!Imports are appropriate for the implementation.
16-34: LGTM! Clean implementation.The
nextmethod correctly implements the sliding window logic with O(1) time complexity. The algorithm properly maintains the window size, updates the running sum, and calculates the moving average.datastructures/streams/moving_average/test_moving_average.py (1)
7-13: LGTM! Comprehensive test cases.The test data covers various scenarios including different window sizes and edge cases.
Describe your change:
Calculate moving average
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.