Refactor functions to complexity 10 or below - #385
Draft
thodson-usgs wants to merge 35 commits into
Draft
Conversation
Extract _check_adapter_known, _check_env_not_refused, _resolve_from_block, _resolve_from_env, and _resolve_from_file from the monolithic _resolve function, giving each tier of the precedence chain its own named, documented helper.
Extract _collect_adapter_overrides and _collect_overrides_for_adapter, separating the collection logic (which carries the nested iteration) from the formatting logic that prints the table.
Extract _ErrorDeduplicatingCell class, _show_file_status, _print_setting_rows, and _show_built_in_default_note, reducing the orchestrator to a short pipeline of named steps.
Extract _check_is_configuration, _check_no_duplicate_adapter, and _add_configuration_overrides from the loop body, leaving _frame as a thin iteration over named validation and rendering steps.
Extract _stat_config_file, _file_cache_valid_by_metadata, _read_file_content, and _parse_or_reuse_cache from the monolithic file-loading function, giving each concern a named, testable unit.
Extract _site_block_boundaries, _build_column_name, _parse_parameter_record, and _parse_site_block from _read_json. Cognitive complexity: 5 (was ~18).
Simplifies format_response by extracting timezone localization into a dedicated helper. Cognitive complexity: format_response 7, helper 5.
Extract _find_datetime_triplets and _resolve_sort_key to reduce _attach_datetime_columns complexity to 2. Note: _find_datetime_triplets is at 14 and will be reduced in a follow-up commit.
…request Reduces _get_features_request cognitive complexity to 9 by extracting lat/long origin-type validation into a dedicated helper (complexity 3).
Reduces search() cognitive complexity to 8 by extracting basin and flowlines dispatch logic into dedicated helpers.
Extract _build_triplet_datetime (complexity 4) to bring _find_datetime_triplets from 14 down to 10.
Extract _format_message() and _resolve_status_code() methods from __init__, moving conditional formatting and cause-chain traversal into focused helpers. No new helper exceeds CC=8. complexipy score: FanOutInterrupted.__init__ 11 → 2
Extract _handle_gather_failures(), _propagate_signals(), _find_first_transient(), and _raise_transient() methods from _run, isolating the failure-precedence logic. No new helper exceeds CC=8. complexipy score: FanOut._run 16 → 3
Extract _validate_file_types(), _validate_time_no_duration(), and _filter_features_by_type() module helpers from get_ratings, separating validation and filtering from the main orchestration. No new helper exceeds CC=8. complexipy score: get_ratings 11 → 4
Extract _extract_features() and _build_outer_frame() module helpers from _handle_nesting, separating feature extraction and outer-frame construction from the main nesting logic. No new helper exceeds CC=8. complexipy score: _handle_nesting 11 → 1
Pre-filter Date columns in the loop iterable, eliminating the filter/continue branch and the first_date_col sentinel check.
Replace the output-format if-chain with a data-driven dispatch dict. ValueError behavior for invalid `to` values is preserved exactly (raised from a KeyError catch).
Extract the inline all(pd.isna(...) or ...) expression in _format_api_dates into a named _all_blank helper. Reduces cognitive complexity from 10 to 8 while preserving exact behavior and types.
Hoist the nested clean() closure in _json_error_detail to a module-level _clean_field helper. The resulting _json_error_detail drops from CC 10 to 7. Behavior and types are unchanged.
Extract the shared try/except httpx.InvalidURL → None pattern from _safe_request_bytes and ChunkPlan._probe_initial_request into a single _try_build helper. Both callers now delegate to it, preserving their original return contracts (int for _safe_request_bytes, tuple for _probe_initial_request). pyscn 1.29.0 clone groups: 9 → 8, no replacement group introduced.
Separate filter-axis construction into _filter_axis (CC 3) and replace the imperative loop with a list comprehension for list axes. _extract_axes cognitive complexity: 11 → 2 (pyscn). No new clone groups introduced (still 8). complexipy still passes at threshold 10.
Filter geometry out of the caller's column list once, then conditionally append it at the end. This replaces the previous include/exclude branches and the separate not-in check with a single strip-then-append pattern, guaranteeing geometry-last order in all paths. Existing test_empty_result_uses_request_geometry_contract asserts the geometry-last column order.
Handle no-explicit-properties (None / all-NaN) as an early-return guard that optionally reorders extra-id columns, then returns immediately. The explicit-properties path becomes the remaining straight-line code with no interleaved condition re-checking properties again.
Post-review cleanup of the CC<=10 refactor; no function exceeds the cap and the offline suite, mypy --strict, and ruff all stay green. - _configuration_core: return the cached parse instead of a validity bool, removing the package's first ``type: ignore`` (the file itself documents carrying none) and restoring the do-not-optimize note the split had dropped. - fanout: merge the failure-precedence trio back into _handle_gather_failures; once every failure is proven transient the first transient is failures[0], so the sentinel accumulator and the package's only ``assert`` go away. - planning: drop _probe_initial_request (its ``fits`` conflated "couldn't build" with "doesn't fit"); move the no-axes rule to a module-level _check_unchunkable_request beside its collaborators; share one key-parameterized largest-chunk scan between the byte pass and the refine pass. - filters: replace the per-character _advance_char tuple threading with _iter_top_level_spaces, a generator that owns the depth/quote state and skips quoted spans via str.find. - configuration: value-returning override helpers instead of out-param mutation; a named mark_reported() instead of poking the dedupe cell's attribute; restore the short-circuiting adapter check (the tuple-splat form allocated on every _resolve call). - shaping: restore main's column-order contract in _empty_feature_frame (geometry stays where a caller-supplied schema put it); the refactor had silently forced geometry last. - states: restore lazy formatting via _format_state (the dispatch dict eagerly built all four renderings per call to discard three). - stats/nldi: drop unused parameters (_extract_features geopd, _validate_lat_long_origin lat/long); make _search_flowlines keyword-only. - _wqx: single-purpose helpers -- _find_datetime_triplets returns only the mapping and _resolve_sort_key derives its own fallback. - dates: drop _coerce_to_list's dead None branch and share one _is_blank predicate between _format_one and _all_blank. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
[tool.complexipy] max-complexity-allowedaccordinglypyscnEach measurable function improvement is preserved as its own commit. All files reached the target; none required the three-no-improvement convergence fallback.
Complexity
Baseline (
complexipy==6.2.0, threshold 10):Final:
_find_datetime_triplets10→8_to_state_one10→7_format_api_dates10→8_json_error_detail10→7_extract_axes10→2_empty_feature_frame10→6_arrange_cols10→7_accepted_keys,_interpret, andconfig_pathremain at 10 after review found their dispatch/caching complexity inherent; extracting it would add indirection and LOC rather than simplify the designBroader health metrics
Comparable
pyscn==1.29.0runs resolved the same 61 modules and 164 dependencies with the correct project root.The first refactor pass introduced two Type-4 clone groups. Both were removed before this update. Coupling, cohesion, dependency depth, and architecture compliance did not regress. The Xenon cyclomatic gate also passes.
wilyshows the expected module-level tradeoff from replacing nested functions with small helpers. Maintainability improves notably in planning, shaping, dates, states, transport fan-out, and the configuration core; requests and filters lose some maintainability index from added helper/LOC overhead, while their target functions become substantially simpler. The package-level structural checks above remain improved.Validation
complexipy dataretrieval --max-complexity-allowed 10 --failed --plain --no-ignorexenon --max-absolute C --max-modules B --max-average A dataretrievalpyscn analyze --json --no-open dataretrievalwily diff dataretrieval --revision 470280d9coverage run -m pytest tests/— 993 passed, 12 live tests deselectedcoverage report -m— 98% total coverageruff check .ruff format --check .mypy dataretrieval— strict, 61 source fileslint-imports— 8 contracts keptgit diff --check