Convert models to macro templates #94
Open
+84,193
−778
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- stripped out old code - added test coverage where needed
Sorry if anybody ever needs this but why on earth did they do this
|
Closes #73 |
|
Closes #63 |
This was
linked to
issues
Oct 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR overhauls the aind-data-schema-models repository to use
jinja2templates.The entry point is the
run_all.shscript, which pulls the templates from the_generators/templatesfolder. The name of each template file is used to get the corresponding data from themodelsfolder. The templates then parse each row, generating the corresponding classes, and merge them into the ALL/ONE_OF patterns that we use. It's not super easy to write the templates so I almost certainly made a typo somewhere, especially in the docstrings appreciate a close eye on them.Some additional utility functions were added to support this and tests to coverage the new code.
Overall I'm happy with this approach, it's super simple and the templates are pretty understandable except for some weird edge cases with how empty columns are treated by pandas. This should be backward compatible with our existing repos, but we need to check carefully before merging this.
Also the
mouse_anatomyfile is massive, although it's only really a few MB so not a big deal to keep it. I vote that we merge this in some form and then revisit how to handle large vocabularies with this issue #80 in the future.