From fb882e09abbaacbff432ef2434a6aec7c1d6f1e7 Mon Sep 17 00:00:00 2001 From: manantlerio <223027124+manantlerio@users.noreply.github.com> Date: Sat, 26 Sep 2026 12:00:37 -0400 Subject: [PATCH] fix(ownership): shares_traded counts zero market trades instead of raising (#1359) `NonDerivativeTable.market_trades` returns None when the non-derivative table has no transactions, and an empty frame when it has transactions but none of them is coded P or S. Every caller of it guards both states - `market_trades` itself in the summary path, `common_stock_purchases`, `common_stock_sales`, `get_ownership_summary` - except `Ownership.shares_traded`, which read `.Shares` straight off it. So a derivative-only Form 4 parsed fine, returned its derivative transaction through `to_dataframe()`, and then raised `AttributeError: 'NoneType' object has no attribute 'Shares'` on the accessor. Vertex Pharmaceuticals' Form 4 filed 2020-10-19 (accession 0001209191-20-055264) is one, and it is already the fixture behind `tests/ownership/test_form4.py::test_form4_with_derivatives`. `shares_traded` now returns 0 for both states. 0 is the count of non-derivative open-market trades and says nothing about derivative activity, which this accessor has never counted - the docstring says so, because the number is easy to misread as "nothing happened". The award-only case already returned 0 by summing an empty numeric column, so that answer is unchanged; the empty branch now also covers an empty frame whose Shares column is not numeric, which fell through to None before. The nonempty non-numeric fall-through to None is deliberately untouched: a filing that traded an amount we cannot parse must not be reported as having traded nothing. A regression test pins that alongside the two controls. The test reads a checked-in copy of the Vertex XML and stubs the reporting-owner CIK lookup, the only fetch in `Ownership.parse_xml`, using the same stub as `tests/ownership/test_insider_parse_contract.py`. Verified offline under `pytest -p tests._offline_harness`, so it gates pull requests rather than landing in the post-merge network lane. Fixes #1359 Co-Authored-By: Claude --- changelog.d/1359.fixed.md | 1 + .../ownership/VertexForm4.derivative-only.xml | 107 ++++++++++++++++++ edgar/ownership/forms.py | 18 ++- ...ssue_1359_shares_traded_derivative_only.py | 104 +++++++++++++++++ 4 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 changelog.d/1359.fixed.md create mode 100644 data/ownership/VertexForm4.derivative-only.xml create mode 100644 tests/issues/regression/test_issue_1359_shares_traded_derivative_only.py diff --git a/changelog.d/1359.fixed.md b/changelog.d/1359.fixed.md new file mode 100644 index 000000000..36fdec062 --- /dev/null +++ b/changelog.d/1359.fixed.md @@ -0,0 +1 @@ +**`Form4.shares_traded` raised `AttributeError` on a derivative-only filing.** `market_trades` is None when the non-derivative table has no transactions, and the accessor read `.Shares` off it without the None/empty guard its neighbours already had: Vertex's 2020-10-19 Form 4 parsed and returned its derivative transaction but crashed on `shares_traded`, which now reports 0 market trades. (GH #1359) diff --git a/data/ownership/VertexForm4.derivative-only.xml b/data/ownership/VertexForm4.derivative-only.xml new file mode 100644 index 000000000..588a865fb --- /dev/null +++ b/data/ownership/VertexForm4.derivative-only.xml @@ -0,0 +1,107 @@ + + + + X0306 + + 4 + + 2020-10-15 + + 0 + + + 0000875320 + VERTEX PHARMACEUTICALS INC / MA + VRTX + + + + + 0001197036 + SACHS BRUCE I + + + C/O VERTEX PHARMACEUTICALS INCORPORATED + 50 NORTHERN AVENUE + BOSTON + MA + 02210 + + + + 1 + 0 + 0 + 0 + + + + + + + Deferred Stock Units + + + + + + 2020-10-15 + + + + 4 + A + 0 + + + + + + + 196.356 + + + 222.81 + + + A + + + + + + + + + + + Common Stock + + + 196.356 + + + + + 12629.042 + + + + + D + + + + + + + Each deferred stock unit represents one share of common stock and is paid out in common stock upon the earliest to occur of (i) termination of Mr. Sachs' service on our board of directors, (ii) a change of control of our company and (iii) Mr. Sachs' disability or death. + + + + + + /s/ Sabrina Yohai, Attorney-in-Fact + 2020-10-19 + + \ No newline at end of file diff --git a/edgar/ownership/forms.py b/edgar/ownership/forms.py index 126828c6c..93c33dc86 100644 --- a/edgar/ownership/forms.py +++ b/edgar/ownership/forms.py @@ -340,9 +340,23 @@ def derivative_trades(self): @cached_property def shares_traded(self): + """Total shares in the non-derivative open-market (P/S) trades. + + ``0`` means this filing reported no such trades. It does not mean the + filing is empty: a derivative-only Form 4 reports its activity through + ``derivative_table`` and still has nothing to count here (GH #1359). + """ + trades = self.market_trades + # `market_trades` is None when the non-derivative table has no + # transactions at all, and empty when none of them is coded P or S. + # Every other caller of it guards both states; this one read `.Shares` + # straight off it and raised AttributeError on a derivative-only + # filing, which parses fine and reports its transaction elsewhere. + if trades is None or trades.empty: + return 0 # Sum the Shares if Shares is all numeric - if np.issubdtype(self.market_trades.Shares.dtype, np.number): - return self.market_trades.Shares.sum() + if np.issubdtype(trades.Shares.dtype, np.number): + return trades.Shares.sum() @classmethod def from_xml(cls, diff --git a/tests/issues/regression/test_issue_1359_shares_traded_derivative_only.py b/tests/issues/regression/test_issue_1359_shares_traded_derivative_only.py new file mode 100644 index 000000000..f67250cb8 --- /dev/null +++ b/tests/issues/regression/test_issue_1359_shares_traded_derivative_only.py @@ -0,0 +1,104 @@ +""" +Regression test for GitHub issue #1359: `shares_traded` on a derivative-only Form 4. + +Vertex Pharmaceuticals Form 4 filed 2020-10-19 (accession 0001209191-20-055264) +reports one derivative transaction and no non-derivative table at all, so +`NonDerivativeTable.market_trades` is None. `Ownership.shares_traded` read +`.Shares` straight off it and raised `AttributeError: 'NoneType' object has no +attribute 'Shares'`, even though the filing parsed and `to_dataframe()` returned +the derivative transaction. Every neighbouring accessor already guarded None. + +GitHub Issue: https://github.com/dgunning/edgartools/issues/1359 +""" +from pathlib import Path + +import pandas as pd +import pytest + +import edgar.ownership.owners as owners_module +from edgar.ownership.forms import Form4 + +# Vertex Pharmaceuticals Form 4, filed 2020-10-19, accession 0001209191-20-055264. +# One derivative transaction (196.356 deferred stock units), no . +DERIVATIVE_ONLY = Path('data/ownership/VertexForm4.derivative-only.xml') +# 374Water Form 4: a non-derivative award (code A) and a derivative award, so the +# non-derivative table has transactions but none of them is a P/S market trade. +AWARDS_ONLY = Path('data/ownership/374WaterForm4.xml') +# Snowflake Form 4: five non-derivative sales, 200,000 shares in total. +MARKET_TRADES = Path('data/form4.snow.xml') +SNOW_SHARES_TRADED = 200_000 + + +@pytest.fixture +def offline_owners(monkeypatch): + """Stub the reporting-owner CIK lookup, the only fetch in this parse path. + + Same stub as `tests/ownership/test_insider_parse_contract.py`: it is what + keeps an `Ownership.parse_xml` assertion in the pull-request gate instead of + the post-merge network lane. + """ + class StubEntity: + def __init__(self, cik): + self.data = type("Data", (), {"is_company": False})() + + monkeypatch.setattr(owners_module, "Entity", StubEntity) + + +def _form4(fixture: Path) -> Form4: + return Form4.parse_xml(fixture.read_text()) + + +def test_derivative_only_filing_reports_zero_shares_traded(offline_owners): + """The reported AttributeError. 0 is the count of P/S market trades, and the + derivative transaction has to remain reachable alongside it.""" + form4 = _form4(DERIVATIVE_ONLY) + + assert len(form4.non_derivative_table.transactions) == 0 + assert len(form4.derivative_table.transactions) == 1 + assert form4.market_trades is None + + assert form4.shares_traded == 0 + + # 0 must not mean the filing was read as empty. + df = form4.to_dataframe() + assert len(df) == 1 + assert df.iloc[0].Shares == 196.356 + + +def test_award_only_filing_still_reports_zero_shares_traded(offline_owners): + """Control for the empty-frame half of the guard: this table has + transactions, so `market_trades` is a frame, but no row is coded P or S.""" + form4 = _form4(AWARDS_ONLY) + + assert form4.non_derivative_table.has_transactions + assert form4.market_trades is not None + assert form4.market_trades.empty + + assert form4.shares_traded == 0 + + +def test_market_trades_are_still_summed(offline_owners): + """Control for the path the guard must not shadow.""" + form4 = _form4(MARKET_TRADES) + + assert len(form4.market_trades) == 5 + assert form4.shares_traded == SNOW_SHARES_TRADED + + +def test_nonnumeric_shares_are_not_reported_as_zero(offline_owners): + """A nonempty frame whose Shares column is not numeric still falls through + to None. Returning 0 there would report "no shares traded" for a filing that + traded an amount we could not read, which is what the empty-frame branch + must not be allowed to shadow. + + The column is forced to `object`, the dtype `np.issubdtype` is reached with + on pandas 2; nothing here depends on how pandas 3 infers a string column. + """ + form4 = _form4(MARKET_TRADES) + unreadable = form4.market_trades.copy() + unreadable['Shares'] = pd.Series( + ['unknown'] * len(unreadable), index=unreadable.index, dtype=object) + assert not unreadable.empty + form4.__dict__['market_trades'] = unreadable + + assert form4.shares_traded is None