Recalculate entire derived time series if necessary (fixes DEV-148)#551
Recalculate entire derived time series if necessary (fixes DEV-148)#551aptiko merged 1 commit intoopenmeteo:masterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1aea0ec19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Fixes derived/auto-processed time series recalculation when the source series is modified non-append-only (e.g., inserts in the middle / deletions), so downstream derived series can be recomputed from scratch when needed (DEV-148).
Changes:
- Introduces an in-memory
has_non_append_modificationsflag onTimeseriesand sets it based on append vs non-append updates. - Passes the flag through the autoprocess enqueue/task pipeline and clears target series data when a full recompute is required.
- Adds/updates tests around the new flag behavior and task invocation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
enhydris/models/timeseries.py |
Adds has_non_append_modifications flag and sets it during data updates. |
enhydris/autoprocess/apps.py |
Enqueues autoprocess task with the non-append flag from the saved timeseries instance. |
enhydris/autoprocess/tasks.py |
Task now accepts the flag and clears target series before executing when a full recompute is needed. |
enhydris/autoprocess/models.py |
AutoProcess save now enqueues execution with full-recompute semantics. |
enhydris/tests/test_models/test_timeseries.py |
Adds unit tests for the new timeseries flag behavior. |
enhydris/autoprocess/tests/test_apps.py |
Updates/adds tests for enqueue behavior and flag propagation. |
enhydris/autoprocess/tests/test_models/test_autoprocess.py |
Updates assertions for the new task signature/arguments. |
enhydris/autoprocess/tests/test_tasks.py |
Adds tests for the task clearing behavior based on the flag. |
Comments suppressed due to low confidence (1)
enhydris/autoprocess/tests/test_tasks.py:29
- Test name
test_not_clears_target_timeseries_when_non_append_modifications_falseis grammatically awkward and reads like a double-negative. Consider renaming to something like “does_not_clear_*” for clarity/consistency with typical test naming.
@mock.patch("enhydris.autoprocess.models.Checks.execute")
def test_not_clears_target_timeseries_when_non_append_modifications_false(self, m):
execute_auto_process(self.auto_process.id, has_non_append_modifications=False)
self.assertEqual(self.target_timeseries.timeseriesrecord_set.count(), 1)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #551 +/- ##
==========================================
+ Coverage 97.42% 97.48% +0.06%
==========================================
Files 55 55
Lines 3529 3542 +13
==========================================
+ Hits 3438 3453 +15
+ Misses 91 89 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aptiko
force-pushed
the
dev-148--recalculate
branch
from
c1aea0e to
b27a9db
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b27a9db451
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aptiko
force-pushed
the
dev-148--recalculate
branch
from
b27a9db to
d8e3935
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8e39354b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
aptiko
force-pushed
the
dev-148--recalculate
branch
from
d8e3935 to
cd6d50b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd6d50b4d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When a derived time series is calculated, only the newer records were being calculated. This failed if data had been inserted in the middle. This is now fixed. If records are deleted, derived time series are also likewise now recalculated.
aptiko
force-pushed
the
dev-148--recalculate
branch
from
cd6d50b to
889531a
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 889531a2b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When a derived time series is calculated, only the newer records were being calculated. This failed if data had been inserted in the middle. This is now fixed. If records are deleted, derived time series are also likewise now recalculated.
Checklist: