Refactor of get chart data endpoint #139
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
src/dashboard/get_chart_data/templates/get_chart_data.sql.jinja
Outdated
Show resolved
Hide resolved
| {%- if stratifier_column %} | ||
| "{{ stratifier_column }}", "{{ data_column }}" | ||
| {%- else %} | ||
| "{{ data_column }}" | ||
| {%- endif %} |
There was a problem hiding this comment.
I will point out here that because you don't care about the prettiness of the resulting SQL (right?), you can instead try to make the jinja code more readable / maintainable.
Things like reducing the duplication here:
{%- if stratifier_column %}
"{{ stratifier_column }}",
{%- endif %}
"{{ data_column }}"
And/or proper indenting throughout for the if/else blocks, plus whatever else. The fact that the jinja in Library is usually hard to read is partly our fault.
There was a problem hiding this comment.
yeah i think we can make some compromises over here that we cant/wont in the library.
There was a problem hiding this comment.
yeah i think we can
makeditch some compromises over here that we cant/wont in the library.
Fixed that for ya 😄
There was a problem hiding this comment.
ok, added a sqlfluff config that didn't aggresively squash things.
src/dashboard/get_chart_data/templates/get_chart_data.sql.jinja
Outdated
Show resolved
Hide resolved
src/dashboard/get_chart_data/templates/filter_numeric.sql.jinja
Outdated
Show resolved
Hide resolved
src/dashboard/get_chart_data/templates/filter_numeric.sql.jinja
Outdated
Show resolved
Hide resolved
src/dashboard/get_chart_data/templates/filter_numeric.sql.jinja
Outdated
Show resolved
Hide resolved
src/dashboard/get_chart_data/templates/filter_numeric.sql.jinja
Outdated
Show resolved
Hide resolved
dogversioning
force-pushed
the
mg/filter-refactor
branch
3 times, most recently
from
87ba837
to
a5041f7
Compare
src/dashboard/get_chart_data/templates/filter_numeric.sql.jinja
Outdated
Show resolved
Hide resolved
src/dashboard/get_chart_data/templates/filter_numeric.sql.jinja
Outdated
Show resolved
Hide resolved
| f'SELECT gender, sum(cnt) as cnt FROM "{TEST_GLUE_DB}"."test_study" ' | ||
| "WHERE COALESCE (cast(race AS VARCHAR)) IS NOT NULL AND gender IS NOT NULL " | ||
| "GROUP BY gender ORDER BY gender", | ||
| f"""SELECT |
There was a problem hiding this comment.
these are getting particularly irritating to maintain - I did open #140 for considering ditching this in favor of an actual in-DB test.
dogversioning
force-pushed
the
mg/filter-refactor
branch
from
a5041f7
to
1aadc98
Compare
dogversioning
force-pushed
the
mg/filter-refactor
branch
from
1aadc98
to
54eba8e
Compare
dogversioning
force-pushed
the
mg/filter-refactor
branch
from
eb13967
to
af76971
Compare
No description provided.