search

timotheeg / tetris_ctm_summary

Script to add metrics to CTM summary and do commentary
MIT License
36 stars 1 forks source link

File organization #4

Open r002 opened 3 years ago

r002 commented 3 years ago

From yobi9's Discord comment on 3/10/21:

File organization: atm, all the files are dumped at the root of the repo: python files, and documentation images. Some folders cleanup would be nice.

r002 commented 3 years ago

Hi @timotheeg ! I made some progress on this issue today. For fun, I decided to finally sit down and try writing a "proper" Config class bc I noticed it was the first TODO that you'd commented in the code 🙂. I've coded in Python for a while now but I've (shamefully 🙄) always essentially done it the way that you did it bc it was always my own side project "weekend-code-warrior" thing 😅. Definitely worked, but very brittle... God help the poor soul who tried accidentally introduced a bogus key into my config.json 😬...

So here's my first attempt: https://github.com/r002/tetris_ctm_summary/blob/d42f61b09c50e71c73545bd0ed3e676aa19946c0/main/config.py

You can run its "test harness" from root with $ python -m main.config

So with this implementation thus far, I've handled two cases:

1. If config.json doesn't exist, it defaults to the hardcoded values:

image

2. If there's a bogus key in config.json, throw an error:

image

So that's progress.

But I'm running to a problem... if someone specifies a config key of the wrong type, the Python code just accepts it: Eg. "print_score_difference": 123123 in config.json yields:

image

Nooooooo.... Curse you, dynamically-typed-language!!! 😤😤😤

What is the easiest way to catch this? Maybe my mind is just too fried but I want to make https://github.com/r002/tetris_ctm_summary/blob/d42f61b09c50e71c73545bd0ed3e676aa19946c0/main/config.py#L46 fail when it tries to populate my Config dataclass with a field of the wrong type. (And preferably with a one-liner; would like to avoid explicitly writing boilerplate if condition type checks in Config if at all possible.) Any suggestions?

Thank you! 🙏🙏🙏

r002 commented 3 years ago

PS. Copy/pasting this musing from the Discord. Contemplating the best "expected behavior" of how the config loading should work:

Also, taking a step back: With the config values: In the spirit of "explicit is better than implicit"-- if a config value is missing (or malformed) in config.json, is it better to:

  1. Exit the program entirely with error message.

  2. Reject config.json entirely (with error message), load all config values from a default set, and continue running the program.

  3. Load well-defined values from config.json but fall back on defaults for any specific missing (or malformed) config values (with error message).

My instinct is (1) or (2) but not (3). Thoughts...? :thinking: