fix(schedule13): get_summary() warns at the caller's line; empty rows display as not reported

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 `<reportingPersons>` now displays its ownership as "not reported" instead of 0, matching the comparison. (GH #1379)

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

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

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 <reportingPersons> displays as not reported, as the comparison treats it, not as 0."""
+    no_rows_xml = re.sub(r'<reportingPersons>.*?</reportingPersons>',
+                         '<reportingPersons></reportingPersons>',
+                         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