diff --git a/CLAUDE.md b/CLAUDE.md index 533dba2f..692ace51 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,6 +94,79 @@ def _duplicate_for_both_directions(limits: pd.DataFrame) -> pd.DataFrame: """ ``` +### Docstrings: generic helpers + +Most helpers don't need this section — a one-line summary and an I/O Example +is right for a helper that names its own columns and serves one caller. +Apply this guide when a helper is *generic*: the signals are that + +- column or key names arrive as parameters instead of appearing literally in + the body, so the body alone can't tell the reader what the data means; +- it has (or is written to invite) several call sites with different domain + meanings; and +- it enforces a rule its callers depend on but don't restate — a precedence + order, a validation contract, a guarantee about the output's shape. + +The first signal is the gate; the other two raise the stakes. For these +functions the docstring is the only place the full story exists, so it earns +a longer, structured treatment — a short narrative, one idea per paragraph, +in this order: + +1. **Summary line: the function's effect, naming the parameters it + connects.** "Resolve which row of ``table`` applies to each key + combination in ``allowed_values``, returning one explicit row per + combination." The summary should earn the function's name — if it is + called _resolve_*, the summary should say what gets resolved. + +2. **The input's story.** Before any mechanism, explain what the input data + is and why it looks the way it does — especially the oddity the function + exists to handle ("Rather than write out a row for every key, the user + may leave a key cell blank…"). Motivating the weirdness is what makes the + mechanism paragraph land. + +3. **What each remaining parameter represents**, in domain terms rather than + shape ("every value the model needs an entry for — e.g. the enabled + expansion elements"). + +4. **The mechanism** — how the function connects the inputs, precise enough + to cover the compound cases (see below). + +5. **The failure contract last** — what happens when the work can't be + completed for some of the data (raise, fallback, or log-and-continue), + and why that response is the right one. + +Wording rules, beyond the general terminology guidance: + +- **Define derived vocabulary from the signature at first mention.** If the + prose says "key columns", pin it down inline: "a key column — any column + of ``table`` not in ``value_columns``". A term defined only by the body's + code is a term the docstring hasn't defined. +- **State behaviour for the compound case, not just the simple one.** "Each + blank is expanded to one row per value" is vague about a row with two + blanks; "one row per combination of ``allowed_values`` across its blank + columns" is not. +- **Phrase rules to cover every case the code handles.** A tie-break + described as "an expanded row vs one that named the key directly" + misdescribes two expanded rows colliding; "where several rows land on the + same key, the one that started with fewer blanks wins" covers both. +- **Borrow concrete names from the I/O Example in the prose** (direction, + path_id). A generic function documented only in generic nouns is hard to + follow; the example's columns give the prose something to point at. +- **Prefer a concrete failing input to set-membership phrasing.** "A + direction of, say, 'sideways' raises" beats "non-blank values must be + among the listed values". +- **Don't let one word do double duty.** "A key column *listed* in + ``allowed_values``" and "among the *listed* values" use one word for two + different lookups. +- **Verify claims about callers and upstream schemas against the code.** + A docstring that states where a parameter's values come from is making a + testable claim — check the call sites first, and describe what callers + actually pass rather than the provenance you assume. + +Worked examples: `_resolve_wildcards` in `src/ispypsa/translator/helpers.py` +(a raising failure contract) and `_fuzzy_match_names` in +`src/ispypsa/templater/helpers.py` (a fallback-and-log contract). + ### Module docstrings A module docstring orients a reader who has never seen the module before. By diff --git a/src/ispypsa/templater/helpers.py b/src/ispypsa/templater/helpers.py index da9456b5..b40ee38f 100644 --- a/src/ispypsa/templater/helpers.py +++ b/src/ispypsa/templater/helpers.py @@ -14,27 +14,42 @@ def _fuzzy_match_names( not_match: str = "existing", threshold: int = 0, ) -> pd.Series: - """ - Fuzzy matches values in `name_series` with values in `choices`. - Fuzzy matching is used where typos or minor differences in names in raw data - may cause issues with exact mappings (e.g. using a dictionary mapping). - This function is only suitable for use where name_series does not have - repeated values since matching is done without replacement + """Replace each name in ``name_series`` with the ``choices`` entry it best + matches, pairing unique names and choices one to one. + + The raw IASR workbooks spell the same entity slightly differently from + sheet to sheet — typos, stray footnotes, punctuation — so a name column + often almost-but-not-exactly matches the canonical names the rest of the + pipeline keys on. An exact dictionary mapping would silently miss those + rows; fuzzy matching repairs them instead. + + ``choices`` holds the canonical spellings (e.g. the NEM sub-region names + hardcoded in mappings.py). ``task_desc`` names the operation in the log line each + repair emits. ``threshold`` (0-100) is the minimum fuzz.ratio score a + pairing must reach; ``not_match`` sets what happens to names left over + when no choice scores that high — the sentinel "existing" keeps the + original name, and any other string is written in as a literal + replacement (e.g. not_match="unknown" stamps unmatched rows "unknown"). + + Matching is one to one without replacement over the unique values of both + sides: an exact match claims its choice first, then the remaining pairs + are matched by repeatedly taking the highest-scoring (name, choice) pair + that reaches ``threshold``. Because each choice can be claimed once, a name may + not receive its own best match if a stronger pairing takes that choice + first — and for the same reason, this function is unsuitable where two + different names in the series should map to the same choice. Nothing + raises: leftover names take the ``not_match`` fallback, and every changed + name is logged at INFO so the repairs can be audited. - Args: - name_series: :class:`pandas.Series` with names to be matched with values in - `choices` - choices: Iterable of `choices` that are replacement values - task_desc: Task description to include in logging information - not_match: optional. Defaults to "existing". If "existing", wherever a match - that exceeds the threshold does not exist the existing value is retained. - If any other string, this will be used to replace the existing value - where a match that exceeds the threshold does not exist. - threshold: match quality threshold to exceed for replacement. Between 0 and 100 - - Returns: - :class:`pandas.Series` with values from `choices` that correspond to the closest - match to the original values in `name_series` + I/O Example: + name_series = ["Centarl NSW", "Southern NSW", "Northern NSW"] + choices = ["Central NSW", "Southern NSW"] + task_desc = "matching sub-region names" + + returns: + ["Central NSW", # typo repaired (logged at INFO) + "Southern NSW", # exact match, claims its choice first + "Northern NSW"] # choices exhausted: "existing" keeps the original """ match_dict = _one_to_one_priority_based_fuzzy_matching( set(name_series), set(choices), not_match, threshold diff --git a/src/ispypsa/translator/helpers.py b/src/ispypsa/translator/helpers.py index 90b57c03..1d63c335 100644 --- a/src/ispypsa/translator/helpers.py +++ b/src/ispypsa/translator/helpers.py @@ -161,23 +161,38 @@ def _resolve_wildcards( allowed_values: dict[str, list], value_columns: list[str], ) -> pd.DataFrame: - """Expand a sparse "wildcard" table into one row per concrete key combination. - - A key column may be left blank (NaN) to act as a wildcard that applies to - every value of that column. ``allowed_values`` lists, for each wildcardable - column, the concrete values it may take — the schema's allowed_values / - allowed_values_from, resolved to actual values. Each blank cell in those - columns is fanned out to every allowed value; a filled cell must itself be - an allowed value — anything else raises. Callers filter their designed - selections (e.g. costs for disabled elements or non-investment-period - years) out before calling, so an out-of-set value reaching this point is - bad input data, not selection. Key columns absent from ``allowed_values`` - (e.g. timeslice) ride along unchanged. - - Once the blanks are filled in, several rows can land on the same key — a - specific row and a wildcard one. The row that used the fewest wildcards (the - most specific) wins; callers rely on the schema's *_resolve_unambiguously - rule to guarantee there is never a tie. + """Resolve which row of ``table`` applies to each key combination in + ``allowed_values``, returning one explicit row per combination. + + Each row of ``table`` assigns values (the ``value_columns``) to a key + (every other column). Rather than write out a row for every key, the user + may leave a key cell blank (NaN) to mean the row applies to every value of + that column — the network schemas call this a wildcard. + + ``allowed_values`` lists, for a subset of the key columns, every value the + model needs an entry for — e.g. the enabled expansion elements, or the + configured investment periods. Together these lists define the full set of + key combinations the output must cover. Callers pass the values the model + run actually uses, which may be narrower than what the table's schema + permits. + + This function resolves the mapping between the two: each row is expanded + to one row per combination of ``allowed_values`` across its blank columns + — a row with only direction blank becomes one row per direction, while a + row with path_id and direction both blank becomes one row per + path_id-direction pair — so every row of the output names its full key + explicitly. Where several rows land on the same key after expansion, the + one that started with fewer blanks wins — a more specific entry overrides + a more general one. The schemas' + *_resolve_unambiguously checks (e.g. limits_resolve_unambiguously) + guarantee no two rows tie. + + Where a row names a key directly instead of leaving it blank, the named + value must be one of that column's ``allowed_values`` — a direction of, + say, "sideways" raises. Callers remove the rows they mean to exclude + before calling, so an unrecognised value here is bad input data, not an + intended exclusion. Key columns not in ``allowed_values`` (e.g. + timeslice) are left untouched. I/O Example: table (a blank cell is a wildcard):