Follow-up from review comments on #549, which renamed SketchQuery → SketchStatistic and PopulationReadout → PopulationStatistic.
The enum renames fixed the type names, but the surrounding vocabulary still uses the old terms:
query: SummaryExpr::SummaryEstimate still has a query: PostAsapSketchStatistic field, and related code still says query (review comment, crates/asap-aware-mapping/src/replacement.rs). Should this be renamed too?readout: maintained-population code still uses readout for locals, fields, and functions after the enum became PopulationStatistic (review comment, crates/asap-aware-mapping/src/maintained_population.rs). Should all of the readout names be renamed?
Proposal
Don't just rename these mechanically. First check what these fields and functions actually do, then pick names that match:
- For each place that uses
query / readout, work out what it does: is it a requested statistic (a description of the result), the act of extracting or estimating from a summary, or an execution step?
- Rename based on that. Something that only describes the requested statistic should line up with
*Statistic. Something that actually performs extraction/estimation may deserve a separate, action-style name (or should stay separate from the statistic type).
- Keep naming consistent across
types, asap-aware-mapping, asap-physical-operators, and the docs.
Relation to #509
This ties into the planner layering in #509 (docs/design_docs/proposals/planner-layering.md). Which names are right depends on which stage/layer owns each concept: the logical description of a requested statistic vs. the physical/execution step that produces it. The renames should follow that staging-and-functionality split, so that each name shows which layer it belongs to.
Follow-up from review comments on #549, which renamed
SketchQuery→SketchStatisticandPopulationReadout→PopulationStatistic.The enum renames fixed the type names, but the surrounding vocabulary still uses the old terms:
query:SummaryExpr::SummaryEstimatestill has aquery: PostAsapSketchStatisticfield, and related code still saysquery(review comment,crates/asap-aware-mapping/src/replacement.rs). Should this be renamed too?readout: maintained-population code still usesreadoutfor locals, fields, and functions after the enum becamePopulationStatistic(review comment,crates/asap-aware-mapping/src/maintained_population.rs). Should all of thereadoutnames be renamed?Proposal
Don't just rename these mechanically. First check what these fields and functions actually do, then pick names that match:
query/readout, work out what it does: is it a requested statistic (a description of the result), the act of extracting or estimating from a summary, or an execution step?*Statistic. Something that actually performs extraction/estimation may deserve a separate, action-style name (or should stay separate from the statistic type).types,asap-aware-mapping,asap-physical-operators, and the docs.Relation to #509
This ties into the planner layering in #509 (
docs/design_docs/proposals/planner-layering.md). Which names are right depends on which stage/layer owns each concept: the logical description of a requested statistic vs. the physical/execution step that produces it. The renames should follow that staging-and-functionality split, so that each name shows which layer it belongs to.