Refactor s2c::get_class_with_defaults

[kfrey-idm]

Reorganizes the get_class_with_defaults to make it somewhat more logical. The function still needs a lot of work in future PRs, but these changes should resolve the bug where default containers aren't unique.

Also enables helpful simplifications planned in EMOD:

• idmType:IReport should be idmAbstractType
• idmType:WaningEffect should be idmAbstractType

Adds additional exceptions and removes some special case handling. Open PRs in emodpy and emodpy-hiv should be merged first. They do not depend on these changes, but will ensure these changes don't break anything:

• Updates and corrections emodpy#46
• Updates and corrections emodpy-hiv#32

No issues (apparently) with emodpy-workflow.

[Copilot]

Pull Request Overview

Refactors the get_class_with_defaults function to improve organization and fix a bug where default containers weren't unique. The changes consolidate the complex conditional logic into a more systematic approach and ensure that default collections are properly isolated between instances.

• Reorganized the conditional logic in get_class_with_defaults into a more structured if-elif chain
• Added a new eval_default helper function to ensure container defaults return new instances rather than shared references
• Removed the show_warnings parameter and some special case handling

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
emod_api/schema_to_class.py	Major refactoring of get_class_with_defaults function with improved logic organization and bug fixes
tests/test_schema.py	New comprehensive test suite for schema validation and unique container defaults
tests/test_config.py	Updated test to use correct EventCoordinator class name

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This is a hardcoded workaround for a specific class. The comment indicates this should be removed, suggesting this is technical debt that should be addressed.

Suggested change

	if ('NodeListConfig' in classname): # KF: Need to remove NodeListConfig
	schema_blob = dict()
	# Removed hardcoded workaround for NodeListConfig

[kfrey-idm]

🥲

[Bridenbecker]

I ran out of time to really review this before I go to ASTMH. comments

• AdditionalRestrictions should be abstract. not sure why it is not
• code starting at 297 looks like it repeats over and over for with mostly the abstract key being different. Seems like that code could be put in a function. I wish there was a way to put the elif check too since it is repeated numerous times.

Sorry not much more

[Bridenbecker]

These variable names make the code hard to read. I forget what key4 is. I'm assuming that you are doing this instead of having the string repeated everywhere. I'd make them constants (i.e. move to module level, make all caps, have better names, etc. If you are trying to have short names, I would do something like "KEY_AR" or "KEY_REPORT". I'd also recommend an indicator in the name like KEY_REPORT_OLD or KEY_REPORT_MH where comment says MH is malaria-hiv

[kfrey-idm]

I've added additional comments so scrolling back and forth isn't needed. Once all the AbstractTypes are correctly labeled in EMOD, most of the if-else chain can go away.

[kfrey-idm]

I ran out of time to really review this before I go to ASTMH. comments

• AdditionalRestrictions should be abstract. not sure why it is not
• code starting at 297 looks like it repeats over and over for with mostly the abstract key being different. Seems like that code could be put in a function. I wish there was a way to put the elif check too since it is repeated numerous times.

Sorry not much more

Thank you! I didn't realize AdditionalRestrictions was abstract. I've changed a few comments to emphasize that it should be abstract.

These changes are setting up future changes. Once IReport, WaningEffect, and AdditionalRestrictions are changed to idmAbstractType in EMOD, this function can be simplified a lot.

For now, there's a lot of if-else to accommodate doing it both ways. That should go away soon, but not in this PR.