csv_overrides: Add stronger type hints#1851
Conversation
auscompgeek
added
code quality
|
|
||
| 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
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7dd742855
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
## Checklist - [ ] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [ ] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [ ] I have not broken the cheatsheet --------- Co-authored-by: Andreas Arvidsson <andreas.arvidsson87@gmail.com>
3 participants
Checklist