Skip to content

Add autogenerated types dict to patch data#110

Merged
Miepee merged 7 commits intomainfrom
typeddict
Jan 31, 2025
Merged

Add autogenerated types dict to patch data#110
Miepee merged 7 commits intomainfrom
typeddict

Conversation

@Miepee
Copy link
Copy Markdown
Contributor

@Miepee Miepee commented Jan 28, 2025

Part 1 of not-sure-how-many that tries to add better typing/cleanup to the patcher data that is currently mostly just dict[Any, Any].

@Miepee Miepee requested a review from biosp4rk January 28, 2025 00:38
@Miepee Miepee changed the title Typeddict Add autogenerated types dict to patch data Jan 28, 2025
Copy link
Copy Markdown
Contributor

@duncathan duncathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems as good a time as any to leave my general feedback here:

  1. I strongly recommend using snake_case keys in the schema for better adherence to the python style guide in the generated TypedDict definitions
  2. the KEY_WHATEVER constants are an antipattern imo. using normal string literals will be better for basically everything. if you want to rename a key, rename it in the generated file with your IDE's rename function and then update the schema
  3. it's a bit surprising that so many keys in your schema aren't required. I find it's best to have as few optional fields as are necessary. I recommend using the ValidatorWithDefault tool we use in ODR, OPR, caver, etc. to automatically fill in defaults before validating the schema. then you can mark any field with a default as required and avoid messing around with get() and whatnot

@Miepee
Copy link
Copy Markdown
Contributor Author

Miepee commented Jan 28, 2025

posted in discord but will post here for posterity too

seems as good a time as any to leave my general feedback here

generally agree with you on these these but would like to address them in seperate PRs.

@Miepee Miepee merged commit d2da61b into main Jan 31, 2025
@biosp4rk biosp4rk deleted the typeddict branch March 7, 2025 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants