Repository navigation
[Python] Document the import-time default of PeriodicImpulse start_timestamp - #40599
Conversation
…mestamp The default start_timestamp of PeriodicImpulse and MatchContinuously is evaluated once, when the module is imported. As discussed in apache#40565 the default stays as is, so document it and point to an explicit timestamp or rebase for starting relative to pipeline execution. Fixes apache#40564
There was a problem hiding this comment.
📋 Review Summary
This PR updates the docstrings for PeriodicImpulse and MatchContinuously to clarify that the default start_timestamp (Timestamp.now()) is evaluated once at module import time rather than when the transform is instantiated or the pipeline starts. It also guides users toward passing an explicit timestamp or using rebase when relative execution timing is desired.
🔍 General Feedback
- The documentation additions are clear, accurate, and adhere to the formatting conventions and line lengths of both files.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40599 +/- ##
=============================================
+ Coverage 56.16% 79.90% +23.73%
=============================================
Files 1124 488 -636
Lines 178336 82384 -95952
Branches 1489 0 -1489
=============================================
- Hits 100170 65830 -34340
+ Misses 75645 16554 -59091
+ Partials 2521 0 -2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @tvalentyn for label python. This pull request likely touches a core component ("core" label). Please review with scrutiny. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
thanks! |
Fixes #40564
As discussed in #40565, the default
start_timestampofPeriodicImpulse(andMatchContinuously, which passes it through) stays as is, but the docstrings didn't say that the default is evaluated once at module import time. This updates both docstrings to say so, and points to passing an explicit timestamp or usingrebaseto start relative to pipeline execution.Docstring-only change, so no CHANGES.md entry.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123orfixes #456).CHANGES.mdwith noteworthy changes. (Not needed, docstring-only.)See the Contributor Guide for more tips on how to make review process smoother.