Open
Conversation
This file contains hidden or 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
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.
Summary
Hey Sabrina! Great work getting the Jinja template set up — you were really close. I just cleaned up a few things that are easy to miss when you're first working with Jinja:
Template file (
yaml_measobs.j2):Tabs vs spaces in YAML: The
{% if %}blocks and some of the lines underneath them were indented with tabs, while the rest of the file used spaces. YAML is very picky about this — mixing the two breaks parsing. I re-indented everything with spaces to match the rest of the file.Escaping literal curly braces: The output YAML needs actual
{and}characters in expressions likeexpr: {phv00123} * 365. But Jinja uses{{ }}for its own variable substitution, so it gets confused when it sees bare braces. The fix is to tell Jinja to output a literal brace using{{ '{' }}and{{ '}' }}. Weird-looking but it works!Indentation of
unit_conversionandunit/rangelines: These were a bit off from where they need to land in the output — I lined them up to match the existing good output files.Python script (section 2B):
Template filename: The
get_template()call referenced"yaml_measob.txt"but the file is actually named"yaml_measobs.j2"(with thes, and.j2extension).trim_blocksandlstrip_blocks: Without these two settings on the JinjaEnvironment, every{% if %}/{% elif %}/{% endif %}line in the template produces a blank line in the output. Addingtrim_blocks=True, lstrip_blocks=Truetells Jinja to swallow those control lines silently.Building the context inside the row loop: The context dictionary was defined once, outside the loop, referencing variables (
phv,bdchm_entity, etc.) that only existed inside the earlier section 2 loop. The fix is to build it from each row as you iterate. Since your template variables already match the CSV column names, we can just pass each row directly withrow.to_dict()— no manual mapping needed!Rendering the template:
yaml.write(context)was writing the Python dictionary object to the file. The Jinja way istemplate.render(**row.to_dict()), which fills in all the{{ }}placeholders and returns the finished string.I tested the output against the existing ARIC reference files (
bdy_wgtfor unit_convert,albumin_bldfor unit_match) and it matches.Test plan
unit_exprpath (if one exists in the current data)