refactor: Phase 1 — slim weather_client_base.py (remove stubs, extract merge helper)#454
Merged
refactor: Phase 1 — slim weather_client_base.py (remove stubs, extract merge helper)#454
Conversation
Deleted 13 one-line _convert_* / _normalize_* / _degrees_to_cardinal / _weather_code_to_description / _format_date_name / _normalize_datetime stubs (lines 1351-1399) that were pure delegation stubs to parsers.* functions. Updated TestWeatherClientHelpers tests to call parsers.* directly instead of going through now-removed WeatherClient stubs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…with parsers imports Removed _convert_f_to_c and _degrees_to_cardinal private methods from VisualCrossingClient (lines 848-876) which duplicated implementations already in weather_client_parsers.py. Extended the parsers import at line 25 and replaced all self._ call sites with direct function calls. Updated helper tests in test_visual_crossing_client.py and test_weather_client_integration.py to call parsers functions directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…in parsers Moved _merge_current_conditions from WeatherClient class body to a standalone merge_current_conditions function in weather_client_parsers.py. The method accessed no instance state — pure function taking two CurrentConditions attrs instances. Added to __all__ and TYPE_CHECKING guard for CurrentConditions. Updated the single call site in _augment_current_with_openmeteo to use parsers.merge_current_conditions(). Re-added weather_client_parsers import alias (had been auto-removed as unused after Task 1 deleted the stubs). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8 test cases covering: primary=None returns fallback, primary wins when populated, fallback fills None fields, fallback fills empty-string fields, both-None stays None, valid object after merge, fallback not mutated, multiple fields merged at once. Co-Authored-By: Claude Sonnet 4.6 <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
Executes Phase 1 of the monolith refactor per
.planning/phases/phase-1/PLAN.md.WeatherClient(_convert_*,_normalize_*,_degrees_to_cardinal,_weather_code_to_description,_format_date_name,_normalize_datetime). These were pure pass-throughs toparsers.*with no logic of their own. Updated 3 test files that were testing the stubs viaclient._*to callparsers.*directly._convert_f_to_cand_degrees_to_cardinalmethods fromVisualCrossingClient(lines 848–876) that duplicated identical implementations inweather_client_parsers.py. Extended the existing parsers import and replaced all 10 call sites._merge_current_conditionsfromWeatherClientto a module-levelmerge_current_conditionsfunction inweather_client_parsers.py. The method was purely functional (noselfstate). AddedTYPE_CHECKINGguard forCurrentConditionsannotation. Re-addedweather_client_parsers as parsersimport toweather_client_base.py(had been auto-removed by ruff after Task 1 dropped the stubs that used it).tests/test_parsers_merge.pywith 8 test cases formerge_current_conditions.Line count changes
weather_client_base.pyvisual_crossing_client.pyweather_client_parsers.pyTest plan
python3 -m pytest --tb=short -q— 2649 passed, 7 pre-existing integration failures (API-key dependent), no regressionsgrep -rn "self\._convert_\|self\._merge_current_conditions" src/accessiweather/— 0 resultspython3 -m ruff checkon touched files — no errorsScope note
Per the plan's honest scope note: after Phase 1,
weather_client_base.pyis ~1,303 lines — not the original <900 target. The unit conversion methods were already stubs (partial extraction had occurred previously). A follow-up phase targeting parse delegation stubs (~250 more lines) is needed to reach <900.🤖 Generated with Claude Code