fix(entity): period_length was accepted, documented, and never read (#1177) - #1198
Merged
Conversation
…1177) `EntityFacts.income_statement(period_length=3)` and `.cash_flow_statement(period_length=3)` returned the ANNUAL statement. The parameter was in both signatures and both docstrings — "3=quarterly, 12=annual" — and referenced zero times in either body; `_build_enhanced_statement()` has no such parameter to receive it. It has never been wired since the Facts API landed in 3b44ade. This is the silent-failure class 6.0 is closing (edgartools-07lk.10, GH #933), in its purest form: a documented input accepted and discarded, wrong data returned, no warning. Filed as edgartools-07lk.10.2 alongside 10.1. Honoured and deprecated in the same release, per the 07lk.23 additive-staging rule. `_resolve_period()` collapses the three spellings into one: period_length=3 -> quarterly, + DeprecationWarning naming period= period_length=12 -> annual, same contradicts period= or annual= -> ValidationError any other value -> ValidationError `period=` defaults to None rather than 'annual' so an explicit request can be told from the default; None still resolves to 'annual', so nothing changes for callers not passing period_length. `cash_flow()` had `annual=True` as a default and forwards period_length, so it would have contradicted itself; its default is now None too. Not a break: there was no way to pass period_length and get what the docstring promised, so no caller can have depended on the old behaviour. Numbers may change for callers who passed it — those numbers were annual figures under a quarterly label. docs/upgrade/6.0.md gains a section at landing time, per the 07lk.18 standard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
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
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.
Fixes #1177. Bead
edgartools-07lk.10.2, underedgartools-07lk.10— the 6.0 unified-error-policy work (GH #933).The defect
EntityFacts.income_statement(period_length=3)and.cash_flow_statement(period_length=3)returned the annual statement. The parameter sat in both signatures and both docstrings — "Optional filter for period length in months (3=quarterly, 12=annual)" — and was referenced zero times in either body (entity_facts.py:1851-1876and2029-2054before this change)._build_enhanced_statement()has no such parameter to receive it. Pergit log -S, it has never been wired since the Facts API landed in3b44ade1.This is the silent-failure class 6.0 is closing, in its purest form: a documented input accepted and discarded, wrong data returned, no warning. Same shape as
07lk.10.1(the hollowFinancials), which shipped in 5.54.0.The fix — honour it, and deprecate it
Staged additively per the
07lk.23rule.EntityFacts._resolve_period()collapses the three spellings for one concept into a single value:period_length=3'quarterly'+DeprecationWarningnamingperiod='quarterly'period_length=12'annual'+ sameperiod=orannual=ValidationError(parameter='period_length')ValidationErrorperiod=ValidationError— IS-AValueError, message unchanged6.0 removes
period_length.period=is the supported spelling and the only one that can also express'ttm'.Two default changes make contradiction detection possible, both behaviour-preserving:
perioddefaults toNonerather than'annual', so an explicit request is distinguishable from the default.Nonestill resolves to'annual'.cash_flow()hadannual=Trueas a default and forwardsperiod_length, so it would have contradicted itself on every call. Its default isNonenow, which resolves identically.Why this is a fix and not a break
There was no way to pass
period_lengthand get what the docstring promised, so no caller can have depended on the old behaviour deliberately. Numbers may change for callers who passed it — those numbers were annual figures under a quarterly label.Verification
tests/issues/regression/test_issue_1177_period_length.py, 25 tests.entity_facts.pymakesperiod_length=3fail for both methods (the builder receivesannual=True), and passes after.period_length=12passes either way — correct, since ignoring it coincidentally gave annual.tests/issues2255 passed / 1 xfailed.docs/upgrade/6.0.mdsection added at landing time, per the07lk.18standard.Note
balance_sheet()never acceptedperiod_lengthand is untouched.🤖 Generated with Claude Code