Drop file-only systemProperties from the investigator node

[eastagiletracker]

This PR proposes dropping the two file-only systemProperties that the investigator node declares but never defines. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/399. You can sign in with your GitHub ID to claim ownership of the project.

What is wrong and what changed

gdcdictionary/schemas/investigator.yaml is an administrative node, but its systemProperties list carries file_state and error_type — the two fields that only data_file / index_file nodes have, and that those nodes define by pulling in _definitions.yaml#/data_file_properties. investigator defines neither, so the node advertises system fields that do not exist in its schema. It also sets additionalProperties: false, so an investigator record could never carry either value in the first place.

Reproduced on master at 918dcd1 with the dictionary as it is loaded by gdcdictionary/__init__.py:

$ python -c "from gdcdictionary import gdcdictionary as g; s=g.schema['investigator']; [s['properties'][k] for k in s['systemProperties']]"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
KeyError: 'file_state'

Any consumer that walks a node's system properties and looks them up in its schema hits that — dictionaryutils' own schema_test.add_system_props does exactly this lookup, so it raises KeyError for any investigator entity rather than validating it. investigator is the only one of the 51 nodes with this mismatch; every other administrative node lists exactly id, project_id, created_datetime, updated_datetime, state.

The change removes those two entries from investigator.yaml and adds test_system_properties_are_defined to gdcdictionary/schema_test.py, which asserts across every node that each entry in systemProperties is defined under properties. On the unmodified schema that test fails with Entity 'investigator' has 'file_state' as a system property but not property; with the fix, all four tests in the module pass. Nothing behavioural is removed: the two fields were unusable on this node.

One thing was needed to make that test runnable at all — the two yaml.load() calls in schema_test.py raise TypeError: load() missing 1 required positional argument: 'Loader' under PyYAML 6, so the module cannot even be imported there; they are now yaml.safe_load(), which behaves the same for these schema files.

Verification, run before and after on the same tree:

# before (clean master)
$ pytest gdcdictionary/schema_test.py -q
ERROR gdcdictionary/schema_test.py - TypeError: load() missing 1 required positional argument: 'Loader'

# after
$ pytest gdcdictionary/schema_test.py -q
4 passed

The uc-cdis/dictionaryutils checks your CI runs against this dictionary are unchanged at 18 passed before and after (tests/test_schema_rules.py, tests/test_required_fields.py, tests/test_required_links.py, tests/test_required_properties.py), and bin/dump_schema.py still emits the same 52-entry artifacts/schema.json, with investigator.systemProperties now listing only the five fields the node actually defines.

How this was managed

This work was tracked as a single story, https://eastagiletracker.com/projects/399/stories/330482, on a board imported from this repository's own issues and pull requests (35 stories) and used to manage the change end to end: https://eastagiletracker.com/projects/399

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com