csv_overrides: Add stronger type hints #1851
Conversation
auscompgeek
added
code quality
|
|
||
| def check_for_duplicates( | ||
| filename: str, default_values: Mapping[str, Mapping[str, str]] | ||
| ): | ||
| results_map = {} |
There was a problem hiding this comment.
I can't see what type this is supposed to be... because I don't think any items are ever assigned to this 😕
I recall this bit was in one of the other functions previously. Think this ended up being broken after a recent refactor.
| file_path = get_full_path(filename) | ||
| super_default_values = get_super_values(default_values) | ||
| assert allow_unknown_values == (default_list_name is not None) |
There was a problem hiding this comment.
Turns out we have dependent types here! It would've been too annoying to check both allow_unknown_values and default_list_name in update_dicts to appease the type checker, but I'm also too lazy to update the callers of init_csv_and_watch_changes right now.
|
I see this is still draft—did you want me to have a look or wanted to make some changes first? |
|
It'd probably make sense to address the above issue first, since as it stands there are no types that are coherent in |
|
Ah right yeah makes sense |
pokey
added
needs-maintainer
Checklist