Repository navigation
Rename query_engine.batch_size to max_channels_per_batch - #108
Merged
Merged
Conversation
…s_per_batch The old name implied a row-count cap, but the setting bounds unique TimeSeriesSelector instances per solve batch. Rename the field throughout the config, report utilities, Report batched solve paths, tests, and documentation. Keep batch_size as a deprecated alias that warns and maps to max_selectors_per_batch; setting both names raises a validation error.
- Copy the input dict in the alias shim so the caller's config is not mutated - Emit FutureWarning instead of DeprecationWarning so users actually see it - Add a deprecated batch_size property/setter forwarding to max_selectors_per_batch - Reject max_selectors_per_batch < 1, mirroring max_containers_per_batch - Wrap two over-long test docstrings
…s_per_batch The setting bounds distinct input channels per solve batch, not generic selectors. Rename the field across config, report utilities, Report batched solve paths, tests, and documentation. Keep `batch_size` as a deprecated alias that now maps to `max_channels_per_batch`; setting both names raises a validation error.
…deprecated batch_size setter - Update `build_batches` and report utilities to deduplicate selectors by `selector_id` rather than object identity, so equivalent channel selections count once. - Allow a single expression whose own inputs exceed `max_channels_per_batch` to run in its own batch instead of failing. - Make the deprecated `batch_size` setter validate the assigned value with the same `>= 1` rule as construction. - Update configuration and API docs to describe the cap as a packing target and clarify selector deduplication behavior. - Add a unit test verifying the deprecated setter rejects non-positive values.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #108 +/- ##
==========================================
+ Coverage 89.80% 89.82% +0.01%
==========================================
Files 62 62
Lines 5740 5770 +30
Branches 722 726 +4
==========================================
+ Hits 5155 5183 +28
- Misses 463 464 +1
- Partials 122 123 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #102.
query_engine.batch_sizelimits how many distinct channel selections are solved per batch, notrows or records. This PR renames it to
max_channels_per_batchso it reads alongsidemax_containers_per_batch.Changes
max_channels_per_batch(default 500, must be>= 1).batch_sizeremains a deprecated alias: it is still accepted in configs and as an attribute,emits a
FutureWarning, and maps to the new field. Setting both is a validation error. It willbe removed in a future release.
report_utilsbatch helpers, tests, docs and skills. The docs now statewhat the cap counts (distinct
channel(...)/poi_channel(...)selections, and for calculatedchannels their input channels) and that it is a packing target: an expression that alone
exceeds it runs in its own batch.
Notes
max_selectors_per_batch), because end users knowchannels, not selectors.
config_hashinmeasurement_dimensionchanges for every report, because the dumped configkey is renamed. The column is written but not read anywhere.
Testing
config_parser_test.py(alias mapping, warning, no mutation of the input config,>= 1validation, deprecated attribute and setter) and
report_utils_test.py.batched_report_pipeline_test.pyandcalculated_channel_test.py.make lintTest Plan
Checklist