Add sec10k metadata directly in PUDL #4035

zschira · 2025-01-28T23:26:42Z

Add metadata for sec10k tables directly to PUDL to solve read-the-docs build issues with pulling metadata from gcs, and make the deltalake dependency unnecessary, which was behaving weirdly.

katie-lamb

Approved

zaneselvans · 2025-01-29T03:14:41Z

I think you may need to set the USE_PUDL_MODELS envvar on the GitHub runner to get the metadata unit tests to pass -- it's finding a bunch of fields which are not being used because the SEC tables aren't included in any of the resources we've defined.

And if it's set in the context of both the unit and integration tests, then the SEC tables would get pulled when it's testing for the merge queue too (assuming the runner has permissions to pull from GCS) which would probably also be good.

zaneselvans

I made a bunch of comments about field names and our existing conventions mostly.

Do we not need to define the resources that these fields are going to be part of in pudl/metadata/resources/sec10k.py

src/pudl/analysis/pudl_models.py

zaneselvans · 2025-01-29T03:30:12Z

src/pudl/metadata/fields.py

+    "block": {
+        "type": "string",
+        "description": "Title of block of data.",
+    },
+    "block_count": {
+        "type": "integer",
+        "description": "Some blocks are repeated, `block_count` defines the index of the data block.",
+    },


What is a data block? More context in these descriptions would be helpful. Is block a specific enough column name?

I think the single backticks will probably not render correctly, since that's markdown formatting, and this will be getting parse as RST in the documentation.

This is basically a narrow tidy table with a bunch of key value pairs that contain company information. These key values are organized in connected blocks. I've changed the naming to hopefully make this structure more clear. Let me know if it makes more sense now.

zaneselvans · 2025-01-29T03:31:25Z

src/pudl/metadata/fields.py

+    "date_filed": {
+        "type": "datetime",
+        "description": "Date filing was submitted.",
+    },


Do we already have a field name that means this, maybe in the context of the FERC XBRL data, which could be re-used here rather than creating a new field name?

I wasn't able to find any fields like that, but I can definitely change this to use an existing field if there's one that makes sense.

zaneselvans · 2025-01-29T03:33:18Z

src/pudl/metadata/fields.py

+    "former_conformed_name": {
+        "type": "string",
+        "description": "Former name of the company.",
+    },


By analogy with company_name and company_name_raw might this be clearer as company_name_former? What does "conformed" mean in this context?

I think that makes sense and I've changed it. I think the conformed_name here comes from the SEC wording, but I don't see any reason to strictly adhere to the terminology they used. @katie-lamb definitely let me know if you're aware of any specific reason to keep conformed in the name.

Yep, former_confermed_name comes directly from a field pulled from the 10-K filing. I don't see any reason to keep conformed though. We don't use this field in any of the modeling bits.

zaneselvans · 2025-01-29T03:34:58Z

src/pudl/metadata/fields.py

+    "form_type": {
+        "type": "string",
+        "description": "Specific version of SEC 10k filed.",
+    },


Above we have exhibit_21_version which seems analogous, but there we say version and here we say type -- if both are versions, and this one is specific to the SEC 10K, it seems like it would be more consistent to use version in both cases. So maybe exhibit21_version and sec10k_version

In our naming conventions we also use the _type suffix to indicate "a human readable category from a well defined list of values"

That makes sense, fixed.

zaneselvans · 2025-01-29T03:50:33Z

src/pudl/metadata/fields.py

+    "state_of_incorporation": {
+        "type": "string",
+        "description": "Two letter state code where company is incorporated.",
+    },


Should this / can this be constrained to match only valid state abbreviations?

I think it should and maybe can. Is there a place where we're applying such a constraint right now? The base state field doesn't have any constraints on it.

zaneselvans · 2025-01-29T03:51:05Z

src/pudl/metadata/fields.py

+    "street_address_2": {
+        "type": "string",
+        "description": "Secondary street address.",
+    },


We already have an address_2 field defined.

zaneselvans · 2025-01-29T03:51:40Z

src/pudl/metadata/fields.py

@@ -4328,6 +4417,10 @@
        "type": "integer",
        "description": "Sub-plant ID links EPA CEMS emissions units to EIA units.",
    },
+    "subsidiary": {


subsidiary_company_name or at least subsidiary_name?

zaneselvans · 2025-01-29T03:52:08Z

src/pudl/metadata/fields.py

+    "value": {
+        "type": "string",
+        "description": "String value of data point.",
+    },


This seems very vague without more context. What does it mean?

Same as above, I've changed the naming for this table to hopefully make it more clear.

zaneselvans · 2025-01-29T03:56:45Z

src/pudl/metadata/fields.py

+    "year_quarter": {
+        "type": "string",
+        "description": "Year quarter filing applies to.",
+    },


I think for all other columns with this kind of information in them, we are using report_date -- sometimes with annual and other times with monthly frequency. In the FERC data I think it also describes quarterly data (although we only pull in Q4 information). Can we store a quarter using a date type, which conveys richer information about the contents, and can be used in conjunction with other report date data?

Converted to report_date

zschira · 2025-01-29T15:16:26Z

Do we not need to define the resources that these fields are going to be part of in pudl/metadata/resources/sec10k.py

@zaneselvans yeah I had just accidentally not added the file in my commit, then got pulled away yesterday before I could fix it. Same with removing the deltalake dependency. Those should both be fixed now and I'll work through the comments of the fields this morning.

katie-lamb · 2025-01-30T00:01:41Z

I left some responses about naming convention stuff but all the updates seem good to me. @zaneselvans I'll let you weigh in to address requested changes.

Add sec10k metadata directly in PUDL

900f926

zschira requested a review from katie-lamb January 28, 2025 23:26

katie-lamb approved these changes Jan 28, 2025

View reviewed changes

zaneselvans assigned zschira Jan 29, 2025

zaneselvans added sec10k Issues related to SEC 10K filing data. and removed mozilla_sec_to_eia Mozilla AI for EJ grant to link SEC utility ownership data to EIA operational data labels Jan 29, 2025

zaneselvans requested changes Jan 29, 2025

View reviewed changes

zschira added 2 commits January 29, 2025 10:05

Remove deltalake dependency

5931d9b

Add missing sec10k resource metadata

31faab3

zschira added 5 commits January 29, 2025 14:26

Improve sec10k schema definitions

26140ec

Override fraction_owned desc for sec tables

90aab59

Improve company_id_sec description

5620521

Convert year_quarters to dates

e2bca00

Only load sec assets if env variable is set

f07f9f9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sec10k metadata directly in PUDL #4035

Add sec10k metadata directly in PUDL #4035

zschira commented Jan 28, 2025

katie-lamb left a comment

zaneselvans commented Jan 29, 2025 •

edited

Loading

zaneselvans left a comment

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

katie-lamb Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zaneselvans Jan 29, 2025

zschira Jan 29, 2025

zschira commented Jan 29, 2025

katie-lamb commented Jan 30, 2025

Add sec10k metadata directly in PUDL #4035

Are you sure you want to change the base?

Add sec10k metadata directly in PUDL #4035

Conversation

zschira commented Jan 28, 2025

katie-lamb left a comment

Choose a reason for hiding this comment

zaneselvans commented Jan 29, 2025 • edited Loading

zaneselvans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zschira commented Jan 29, 2025

katie-lamb commented Jan 30, 2025

zaneselvans commented Jan 29, 2025 •

edited

Loading