From 617410c9c81e94db6f914daeccda59acfd5f3ff2 Mon Sep 17 00:00:00 2001 From: Dwight Gunning Date: Wed, 30 Sep 2026 10:52:40 -0400 Subject: [PATCH] fix(schedule13): get_summary() warns at the caller's line; empty rows display as not reported Follow-up to #1379. get_summary() read self.shares_change and self.percent_change, so it emitted two FutureWarnings whose stacklevel landed inside amendments.py. Python's default filter keys on that location, so the warning showed once per process and never named the user's call site. get_summary() now computes the 5.x values directly and emits one warning of its own at the caller's line. _reported_total() returned max([], default=0) for a filing with no reporting-person rows, so to_context() and the Rich tables showed 0 shares while OwnershipComparison treated the same filing as unknown. It now returns None, and the display says "not reported". Co-Authored-By: Claude Opus 5.5 --- changelog.d/1379-summary-warning.fixed.md | 1 + edgar/beneficial_ownership/amendments.py | 37 ++++++++++++++---- edgar/beneficial_ownership/models.py | 4 ++ ...ssue_1379_schedule13_unreported_figures.py | 38 +++++++++++++++++++ 4 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 changelog.d/1379-summary-warning.fixed.md diff --git a/changelog.d/1379-summary-warning.fixed.md b/changelog.d/1379-summary-warning.fixed.md new file mode 100644 index 000000000..a26aac2ba --- /dev/null +++ b/changelog.d/1379-summary-warning.fixed.md @@ -0,0 +1 @@ +**`OwnershipComparison.get_summary()` warnings now point at the caller.** It emitted two `FutureWarning`s attributed to `amendments.py`, so Python showed them once per process wherever it was called from; it now emits one, at the caller's line. A filing with an empty `` now displays its ownership as "not reported" instead of 0, matching the comparison. (GH #1379) diff --git a/edgar/beneficial_ownership/amendments.py b/edgar/beneficial_ownership/amendments.py index e72062d7a..a261addb2 100644 --- a/edgar/beneficial_ownership/amendments.py +++ b/edgar/beneficial_ownership/amendments.py @@ -81,6 +81,7 @@ def _placeholder_sum(schedule: 'Schedule13D | Schedule13G', figure: str): def _warn_unreported(name: str, replacement: str) -> None: # The text is fixed per property so Python's default filter shows it once per call site. + # stacklevel=3 skips this helper and the property, so the caller's line is reported. warnings.warn( f"OwnershipComparison.{name} is computed from figures a filing does not report, read as 0. " f"It will be None in edgartools 6.0. Use OwnershipComparison.{replacement}, which is None today.", @@ -89,6 +90,18 @@ def _warn_unreported(name: str, replacement: str) -> None: ) +def _warn_unreported_summary() -> None: + # get_summary() warns for itself: reading self.shares_change would attribute the + # warning to this module, and the default filter would then show it once per process. + warnings.warn( + "OwnershipComparison.get_summary() computes 'shares_change' and 'percent_change' from figures " + "a filing does not report, read as 0. They will be None in edgartools 6.0. " + "Use the 'reported_shares_change' and 'reported_percent_change' keys, which are None today.", + FutureWarning, + stacklevel=3, + ) + + @dataclass class OwnershipComparison: """ @@ -203,17 +216,25 @@ def get_summary(self) -> dict: emit the same ``FutureWarning``; ``reported_shares_change`` and ``reported_percent_change`` are None for them. """ + reported_shares_change = self.reported_shares_change + reported_percent_change = self.reported_percent_change + if reported_shares_change is None or reported_percent_change is None: + _warn_unreported_summary() + previous_shares = _placeholder_sum(self.previous, 'aggregate_amount') + current_shares = _placeholder_sum(self.current, 'aggregate_amount') + previous_percent = _placeholder_sum(self.previous, 'percent_of_class') + current_percent = _placeholder_sum(self.current, 'percent_of_class') return { 'previous_filing_date': self.previous.filing_date, 'current_filing_date': self.current.filing_date, - 'previous_shares': _placeholder_sum(self.previous, 'aggregate_amount'), - 'current_shares': _placeholder_sum(self.current, 'aggregate_amount'), - 'shares_change': self.shares_change, - 'previous_percent': _placeholder_sum(self.previous, 'percent_of_class'), - 'current_percent': _placeholder_sum(self.current, 'percent_of_class'), - 'percent_change': self.percent_change, - 'reported_shares_change': self.reported_shares_change, - 'reported_percent_change': self.reported_percent_change, + 'previous_shares': previous_shares, + 'current_shares': current_shares, + 'shares_change': current_shares - previous_shares, + 'previous_percent': previous_percent, + 'current_percent': current_percent, + 'percent_change': current_percent - previous_percent, + 'reported_shares_change': reported_shares_change, + 'reported_percent_change': reported_percent_change, 'is_accumulating': self.is_accumulating, 'is_liquidating': self.is_liquidating, 'is_unchanged': self.is_unchanged diff --git a/edgar/beneficial_ownership/models.py b/edgar/beneficial_ownership/models.py index f3e669bd6..ba38059ba 100644 --- a/edgar/beneficial_ownership/models.py +++ b/edgar/beneficial_ownership/models.py @@ -37,7 +37,11 @@ def _reported_total(persons, figure: str) -> Optional[Union[int, float]]: The same max() over the persons not flagged ``is_aggregate_exclude_shares`` that ``total_shares`` and ``total_percent`` take, which read an unreported figure as 0. + A filing with no reporting-person rows reports no figure either, matching + ``OwnershipComparison.reported_shares_change``. """ + if not persons: + return None values = [p.reported(figure) for p in persons if not p.is_aggregate_exclude_shares] if None in values: return None diff --git a/tests/issues/regression/test_issue_1379_schedule13_unreported_figures.py b/tests/issues/regression/test_issue_1379_schedule13_unreported_figures.py index a399a169d..2e605089a 100644 --- a/tests/issues/regression/test_issue_1379_schedule13_unreported_figures.py +++ b/tests/issues/regression/test_issue_1379_schedule13_unreported_figures.py @@ -415,3 +415,41 @@ def test_ownership_comparison_13g_amendment_reporting_figures(): assert comparison.percent_change == pytest.approx(-2.0) assert comparison.is_liquidating is True assert amendment.total_shares == 8_000_000 + + +@pytest.mark.fast +def test_get_summary_warns_once_at_the_callers_line(): + """get_summary() names the caller's line, not amendments.py. + + It used to read self.shares_change and self.percent_change, so it emitted two + warnings attributed to amendments.py, and Python's default filter then showed + them once per process no matter where get_summary() was called from. + """ + comparison = OwnershipComparison(current=_schedule13d_from_xml(SCHEDULE_13D_XML_PATH.read_text()), + previous=_header_only_schedule13d()) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + summary = comparison.get_summary() + + future = [w for w in caught if issubclass(w.category, FutureWarning)] + assert len(future) == 1 + assert future[0].filename == __file__ + assert "'reported_shares_change'" in str(future[0].message) + # The 5.x values are unchanged. + assert summary['shares_change'] == 4_535_000 + assert summary['current_shares'] == 4_535_000 + assert summary['previous_shares'] == 0 + assert summary['reported_shares_change'] is None + + +@pytest.mark.fast +def test_no_reporting_person_rows_display_ownership_as_not_reported(): + """An empty displays as not reported, as the comparison treats it, not as 0.""" + no_rows_xml = re.sub(r'.*?', + '', + SCHEDULE_13D_XML_PATH.read_text(), flags=re.S) + no_rows = _schedule13d_from_xml(no_rows_xml) + + context = no_rows.to_context() + assert 'Ownership: percent not reported (shares not reported)' in context + assert '0 shares' not in context