Repository navigation
fix(stocks): build INTERNAL quotes that carry the 52-week range - #157
Conversation
StockQuote and StockQuotesHumanReadable take the API's 52weekHigh / 52weekLow (52 Week High / 52 Week Low) as fiftyTwoWeekHigh / fiftyTwoWeekLow (Fifty_Two_Week_High / Fifty_Two_Week_Low), Decimal money, None when the call did not ask for them. The empty result of a no_data answer carries the two columns only under use_52_week=True, the shape a populated answer has.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #157 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 62 62
Lines 3196 3204 +8
=========================================
+ Hits 3196 3204 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ Ready — Code reviewThis fixes INTERNAL stock quotes with Rating patch 6/6 (exemplary) · proof 6/6 (comprehensive) · overall 6/6 Since last review
Reviewed |
… low The live check takes each bound as a positive Decimal or None, and checks low <= high only when both are present: the API answers null when it cannot derive the range. A 52weekLow of 0, which the API answers before the open (MarketData-App/api#474), is recorded as an expected failure instead of failing the job.
#157 conflicted in two places: the imports of resources/base.py keep both csv and Collection, and StockQuote.from_dict keeps this branch's _split_fields and _with_extra, with its docstring.
Closes #102
Summary
stocks.quotes(use_52_week=True)fails onOutputFormat.INTERNAL: the API adds52weekHighand52weekLow(52 Week Highand52 Week Lowunderhuman=true), and neither quote model has a field for them. Each model now takes them in two optional money fields, named as in sdk-go:StockQuote52weekHigh,52weekLowfiftyTwoWeekHigh,fiftyTwoWeekLowStockQuotesHumanReadable52 Week High,52 Week LowFifty_Two_Week_High,Fifty_Two_Week_LowBoth are
Decimal | None, andNonewhen the call did not ask for them.Version impact
minor (1.3.0 -> 1.4.0). The PR adds public names and removes or narrows none: the two fields on each quote model,
StockQuote.api_names,fifty_two_week_fieldson both models, and the optionalomit_fieldsargument ofno_data_result. 1.3.0 is the last release and the version inpyproject.tomlonmain. The next release is 2.0.0 either way, sincemainalready carries breaking changes. The CHANGELOG entry stays under Fixed. What a caller sees change:use_52_week=Truereturns the quotes instead of raising.no_dataanswer has the two columns in its DataFrame and CSV header only when the call asked for them. That is the shape a populated answer has in both cases. Before, ause_52_week=Truecall got an empty frame without them.float64where it wasint64, as in every other price column.Nothing changes for a call without
use_52_week.The change
output_types/stocks_quotes.py:api_names, since no field can be named52weekHigh.StockQuote.from_dictrenames the keys with_to_fields, as its human twin already did.fifty_two_week_fields.resources/base.py:no_data_resulttakesomit_fields, the fields the answer would not carry, and leaves their columns out of the empty DataFrame and CSV header. Its docstring now lists its arguments. Its two comments on the JSON branch lose their issue numbers and history, and each keeps the invariant it guards in one line.resources/stocks/quotes.py: omitsfifty_two_week_fieldsunlessuse_52_weekis set. The column names stay inapi_namesonly.docs/stocks.mdlists the four fields and says whatuse_52_weekadds.Out of scope: any other key the models do not declare still raises
ParseError. That policy is #111.Proof
What main does. The issue says the error is a bare
TypeError. Sincemodel_errorsit is aParseError, and it still fails. On mainbc0619c, with a mocked answer that carries the two keys:DataFrame output on main already worked. It had the two columns last, and
52weekLowwasint64for a whole-number value.What the API sends, checked live on 2026-09-28 with the market open: the CSV headers at 14:51Z and the JSON keys at 14:52Z. With
52week=truethe two keys come afterupdated(Date), in JSON and in CSV, plain and human. Without it, neither is sent. The CSV headers ofAAPL:Suite. main
bc0619c: 1954 passed. This branch: 1970 passed, the same 1954 plus the 16 new tests. Two existing tables gained the new names because they list every money field and every human column by hand:MONEY_FIELDSintest_money_decimal.pyandAPI_HUMAN_COLUMNSintest_output_types.py. No other existing test changed.The new tests fail on main. With main's three source files checked out on this branch, 10 of the 16 fail:
52week;52week.The 6 that pass are the cases without
52week, which pin what does not change.Mutation.
quotes.pynever omits the fields: 10 fail. That is the four cases of the existingtest_every_resource_no_data_dataframe_has_the_shape_of_a_populated_one[stocks_quotes-*], plus the six new cases without52week. Restored: all pass.Live.
src/tests/integration/test_stocks_live.py -k quoteson 2026-09-28 at 14:51Z: 3 passed, the two new ones included. The new test again onda96b09at 21:41Z: 2 passed. Before the 09:30 ET open the API answers52weekLow: 0forAAPLwhile52weekHighkeeps the year's value (MarketData-App/api#474): the test onf6857fcfailed on that answer at 12:04Z on 2026-09-28, in attempt 2 of CI run 36158589796. Sinceda96b09the test records that answer as an expected failure and checks every other answer in full, so its result does not depend on the time of day.Tests
src/tests/test_stocks_quotes.py, plain and human:test_internal_quotes_carry_the_52_week_range_when_asked_forsends52week=trueand getsDecimalvalues, a whole-number price included (169isDecimal("169")).test_internal_quotes_hold_no_52_week_range_when_not_asked_forgetsNonein both fields.src/tests/test_status_mapping.py:test_the_empty_quotes_frame_has_the_52_week_columns_only_when_asked_for, on pandas and polars, plain and human, with and without52week: the empty frame has the index, columns, order and dtypes of a populated one, and has52weekLowonly when it was asked for.test_the_empty_quotes_csv_has_the_52_week_columns_only_when_asked_for: the four headers, written by hand from the live answers above.src/tests/integration/test_stocks_live.py:test_quotes_carry_the_52_week_range_when_asked_for, plain and human. INTERNAL output withuse_52_week=Trueholds each bound as a positiveDecimalorNone, since the API answers null when it cannot derive the range, withlow <= highwhen both are present. A52weekLowof0is anxfailthat names MarketData-App/api#474.Review round 1
Measured on 2026-09-28, on
f6857fc. No code changes in this round, so the head and its CI stay as they are.795e86fe: the version impact is minor. Version impact now readsminor (1.3.0 -> 1.4.0). 1.3.0 is the last release (v1.3.0) and the version inpyproject.tomlonmain. The CHANGELOG entry stays under Fixed, as the review allows.Proof timestamps. The live answers quoted in Proof were captured again on 2026-09-28 with the market open: the CSV headers at 14:51Z and the JSON keys at 14:52Z. The four CSV headers are the ones Proof quotes, and the JSON keys come in the order it describes. The live
-k quotesrun at 14:51Z: 3 passed.The live test depends on the time of day until MarketData-App/api#474 is fixed. Before the open the API answers
52weekLow: 0forAAPL, so0 < low <= highfails. The times are under Live in Proof. The test is unchanged in this round.Review round 2
Measured on 2026-09-28, on
da96b09: one commit on top off6857fcthat changes only the live test.7a4eaf48: the live test no longer depends on the time of day. In order, the test checks:Noneor aDecimal. This comes first, so a mapping that returned a float or an int still fails at any hour;52weekLowof0, the API's answer before the open (MarketData-App/api#474), is anxfail;low <= highwhen both are.Noneis accepted because the API answers null when it cannot derive the range. Run against a stub client in both parametrizations, the range(345.34, 242.7592),(None, None)and a singleNonepass; a0low and(0, 0)xfail;low > high, a negative bound, a float0.0low and float bounds fail. The body onf6857fcfails the0low case. Live at 21:41Z: 2 passed.bf4373dc: no sibling SDK builds its own empty result for stock quotes, so none has this defect. Read on each default branch on 2026-09-28. Onno_dataeach one returns the API's own body (CSV and HTML) or an empty typed result, and none builds CSV columns from its model:901e855:stocks.CSVService.QuoteandQuotesgo throughFetchFormatted, which wraps the raw body; the typedQuotesreturns an empty slice withNoData.7995952: CSV returns the API'sBlobas it came (no_datais an emptyBlob); JSON returns[]with theno_dataflag.ec5181b: CSV and HTML return the body under->csvand->html; the JSONQuotekeepsstatus = 'no_data'with null fields.d6f1077:StocksCsvResource.quoteandquotesreturn the API's text as aCsvResponse.cb83a0f:GetQuoteCsvAsyncreturns the raw payload throughCreateCsvResponse.Each typed quote model already has the two 52-week fields. No sibling issue is needed.
Checks on
da96b09.uv run pytest -n 4: 1970 passed; the live suite is opt-in, so the count does not change../lint.shclean.git merge-treeis clean againstmain, #154, #158 and #160. Against #159 it conflicts inStockQuote.from_dict, which #159 now builds with_split_fieldsand_with_extra, and in thecollections.abcimport line ofresources/base.py; both conflicts are already there onf6857fc.